Themed default speed signs #1320

originalfoo · 2022-01-26T22:53:32Z

This is based on user-feedback resulting from 11.6.4.0 STABLE release #1221 (comment)

This PR adds a new mod option, Options.differentiateDefaultSpeedsInNormalView, which is false by default. It is modified via a CheckboxOption on the General options tab:

In normal view / persistent overlays:

false: themed icons depict default speeds
true: unthemed icons depict default speeds

In defaults view:

Unthemed icons always used (existing functionality)

Also fixes the issue with overly large lane icons #1221 (comment)

Screenshots...

Normal mode, option is false (default state):

Normal mode, option is true:

Defaults mode, regardless of option state:

Persistent overlay, option is false:

Persistent overlay, option is true:

Todo:

Localisations

Possible future enhancements:

Overlay a small green circle (filled) at bottom of sign to indicate it's default speed limit?

It's placed under existing speed theme stuff on general options tab.

In normal view / persistent overlays: - Default off, so themed icons shown for default speeds - When on, unthemed icons will be shown for default speeds In defaults view: - Unthemed icons always used (existing functionality)

TLM/TLM/UI/SubTools/SpeedLimits/Overlay/SpeedLimitsOverlay.cs

TLM/TLM/State/OptionsTabs/OptionsGeneralTab.cs

originalfoo · 2022-01-27T08:04:51Z

Might have got a little carried away ...and refactored the segment and lane overlay code. Found what was causing the weird sizing of lane icons so fixed that too. Also, LanePos struct now has finalDirection field which is probably useful in lots of code that uses GetSortedLanes.

Hopefully speed limit overlays are a little faster but I'm not sure how to properly benchmark stuff.

Note: I've not tested the monorail stuff (where it adds an extra icon on roads which contain monorail).

TLM/TLM/Manager/Impl/OptionsManager.cs

originalfoo · 2022-01-28T03:22:27Z

@kvakvs In ShowSigns_GUI(), what is the purpose of bool hover - it never seems to get used for anything...? Can it be removed or were there plans to do something with it?

EDIT: I guess if we know mouse hovering an icon in one segment, we don't need to check for hover in any other segments? But rather than passing it around through method returns, it would be better to add to drawEnv?

originalfoo · 2022-01-28T14:47:31Z

Goal to beat:

11.6.4.2 - Render latency: 23 ms

kvakvs · 2022-01-28T14:49:54Z

@kvakvs In ShowSigns_GUI(), what is the purpose of bool hover - it never seems to get used for anything...? Can it be removed or were there plans to do something with it?

I think this was used to add to the hovered boxes list on render, but later the code was moved deeper into sub-functions.

krzychu124 · 2022-01-28T15:04:25Z

Goal to beat:

11.6.4.2 - Render latency: 23 ms

how did you measure that? Stopwatch around the method call or something different? (I usually use ThreadProfiler because of builti-in auto avg time, peak per 64 calls, last call time - nicely browsable with ModTools)

- Most fields in `DrawEnv` now `private set` properties - Some logic moved to new `ResetCacheIfNecessary()` - Simplified params of the segment/lane rendering methods - Cull excess `ContainsMouse()` checks where possible - Reduced instantiations of `OverlayHandleColorController()` - Removed some old `hover` code

originalfoo · 2022-01-28T20:10:10Z

how did you measure that? Stopwatch around the method call or something different? (I usually use ThreadProfiler because of builti-in auto avg time, peak per 64 calls, last call time - nicely browsable with ModTools)

I used nvidia Alt+R overlay which shows total latency for the scene ( game + TM:PE stuff ). Managed to get up to 250ms latency at one point 😂 so did some chainsawing and now it's running about 18-20ms latency. Obviously not a very accurate measure, but at least the framerate isn't tanking like it was earlier today.

If you want to do proper test your way - 11.6.4.2 vs. this PR - that would be much appreciated; for all I know I might still have made it slower than it was before.

I'm going to re-ponder what best field name & text for the new mod option should be, then I'll set that up on Crowdin.

Note there's a blocking PR required to be merged prior to this one; #1321

originalfoo · 2022-01-28T20:17:01Z

TLM/TLM/UI/SubTools/SpeedLimits/Overlay/SpeedLimitsOverlay.cs


-                // Ignore: Bad segments
-                if (!segment.IsValid()) {


Removed this because it's already included in segment.MayHaveCustomSpeedLimits()

originalfoo · 2022-01-28T20:18:28Z

TLM/TLM/UI/SubTools/SpeedLimits/Overlay/SpeedLimitsOverlay.cs

-        /// <param name="segCenter">Bezier center for the segment to draw at.</param>
-        /// <param name="camPos">Camera.</param>
-        /// <param name="args">Render args.</param>
-        private bool DrawSpeedLimitHandles_PerSegment(ushort segmentId,


This wrapper method became superfluous after refactoring so removed it.

Glad you're having fun 😀

I couldn't find any references to it so hopefully it doesn't break anything heh.

krzychu124 · 2022-02-13T19:10:57Z

I assume this will also go in 11.6.5 as planned?

originalfoo · 2022-02-13T20:14:41Z

I assume this will also go in 11.6.5 as planned?

@krzychu124 I'm not sure now. Maybe a better approach would be to always show themed signs in normal/overlay mods but with a "green dot" sub-icon if the speed is same as default for the network. I think that would lead to much smaller code change and be better for end-users.

krzychu124 · 2022-02-13T20:21:20Z

Question is if they want to know if speed is different than default🤔

originalfoo · 2022-02-13T20:35:47Z

Question is if they want to know if speed is different than default🤔

Well, if we're adding green blob to bottom of themed sign, it will indicate that segment/lane is using default speed.

No green blob = fully custom speed.

We could flip it the other way round, eg. show some sort of "customised" indicator on the non-default speed icons, but I'm not sure what that icon could look like.

krzychu124 · 2022-02-13T20:43:46Z

Question is if they want to know if speed is different than default🤔

Well, if we're adding green blob to bottom of themed sign, it will indicate that segment/lane is using default speed.

No green blob = fully custom speed.

Not bad idea, maybe it won't be annoying for users like green signs in US/UK theme or others where signs are not round - you know.. AN (replace speed signs, add stop/yield signs), full immersion 😄

I assume that "defaults speeds mode" remains as is - full green sings?

originalfoo · 2022-02-13T20:48:00Z

Yes, default speeds will remain with unthemed signs.

I'll have a go later tonight at doing a smaller PR that does the green blobs thing.

I think ideally we need a separate OverlayManager to centralise things like node/segment caches, determination of what mouse is hovering in terms of nodes/segments, persistent overlays (including dev overlays which are currently buried in non-related classes).

krzychu124 · 2022-02-13T20:55:23Z

I think ideally we need a separate OverlayManager to centralise things like node/segment caches, determination of what mouse is hovering in terms of nodes/segments, persistent overlays (including dev overlays which are currently buried in non-related classes

Yup, I already have some scraps for most performant access to node/segment/vehicles on screen in PF-Sandbox mod. I just need a bit of free time to glue up everything into single overlay manager

originalfoo · 2022-02-14T05:21:52Z

Closing in favor of #1404

originalfoo added 2 commits January 26, 2022 21:33

Add DifferentiateDefaultSpeedsInNormalView option

4c097cb

It's placed under existing speed theme stuff on general options tab.

Make the option take effect

c66f914

In normal view / persistent overlays: - Default off, so themed icons shown for default speeds - When on, unthemed icons will be shown for default speeds In defaults view: - Unthemed icons always used (existing functionality)

originalfoo added Localisation Localised text and features DO NOT MERGE YET Don't merge this PR, even if approved, until further notice SPEED LIMITS Feature: Speed limits Settings Road config, mod options, config xml labels Jan 26, 2022

originalfoo added this to the 11.6.5 milestone Jan 26, 2022

originalfoo requested review from kvakvs, krzychu124, kianzarrin and chameleon-tbn January 26, 2022 22:53

originalfoo self-assigned this Jan 26, 2022

originalfoo commented Jan 26, 2022

View reviewed changes

TLM/TLM/UI/SubTools/SpeedLimits/Overlay/SpeedLimitsOverlay.cs Outdated Show resolved Hide resolved

originalfoo commented Jan 26, 2022

View reviewed changes

TLM/TLM/State/OptionsTabs/OptionsGeneralTab.cs Outdated Show resolved Hide resolved

Refactor segment/lane speed icon overlay

491481b

kvakvs reviewed Jan 27, 2022

View reviewed changes

TLM/TLM/Manager/Impl/OptionsManager.cs Outdated Show resolved Hide resolved

Minor optimisation

3547610

This was referenced Jan 27, 2022

Future ideas for speed limit tool (scope creep from PR 1168) #1221

Open

Refactor OptionsManager.cs #1321

Merged

originalfoo added the code cleanup Refactor code, remove old code, improve maintainability label Jan 27, 2022

Merge branch 'master' into themed-default-speed-signs

3ded530

originalfoo added 2 commits January 28, 2022 19:49

Add missing check for deleted roads

f3ea95f

originalfoo commented Jan 28, 2022

View reviewed changes

originalfoo added 6 commits January 29, 2022 04:21

Merge branch 'master' into themed-default-speed-signs

e894918

Merge branch 'master' into themed-default-speed-signs

7d62ec5

Merge branch 'master' into themed-default-speed-signs

90dd1a9

Fix typo that was breaking CI builds

466427e

Invert the option (so it's now Use... rather than Don't use)

0b66681

Fix byte array size

c4fca21

originalfoo mentioned this pull request Feb 1, 2022

Translations for 11.6.5 (localisation / locale / crowdin) #1344

Closed

9 tasks

Merge branch 'master' into themed-default-speed-signs

203d28d

originalfoo removed this from the 11.6.5 milestone Feb 13, 2022

originalfoo mentioned this pull request Feb 14, 2022

Themed default speed signs (redux) #1404

Merged

4 tasks

originalfoo added this to the Not Applicable milestone Feb 14, 2022

originalfoo closed this Feb 14, 2022

originalfoo deleted the themed-default-speed-signs branch February 14, 2022 05:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Themed default speed signs #1320

Themed default speed signs #1320

originalfoo commented Jan 26, 2022 •

edited

Loading

originalfoo commented Jan 27, 2022

originalfoo commented Jan 28, 2022 •

edited

Loading

originalfoo commented Jan 28, 2022

kvakvs commented Jan 28, 2022

krzychu124 commented Jan 28, 2022

originalfoo commented Jan 28, 2022

originalfoo Jan 28, 2022

originalfoo Jan 28, 2022

kvakvs Jan 28, 2022

originalfoo Jan 28, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 14, 2022

Themed default speed signs #1320

Themed default speed signs #1320

Conversation

originalfoo commented Jan 26, 2022 • edited Loading

originalfoo commented Jan 27, 2022

originalfoo commented Jan 28, 2022 • edited Loading

originalfoo commented Jan 28, 2022

kvakvs commented Jan 28, 2022

krzychu124 commented Jan 28, 2022

originalfoo commented Jan 28, 2022

originalfoo Jan 28, 2022

Choose a reason for hiding this comment

originalfoo Jan 28, 2022

Choose a reason for hiding this comment

kvakvs Jan 28, 2022

Choose a reason for hiding this comment

originalfoo Jan 28, 2022

Choose a reason for hiding this comment

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 13, 2022

krzychu124 commented Feb 13, 2022

originalfoo commented Feb 14, 2022

originalfoo commented Jan 26, 2022 •

edited

Loading

originalfoo commented Jan 28, 2022 •

edited

Loading