Refactor features, overlays, and junction option groups #1422

originalfoo · 2022-02-18T02:09:58Z

Compiled mod for testing: TMPE.zip

Part of ongoing work for #1356 phase 2 - #1356 (comment)

This started out as just a refactor of the Activated Features option group, but it was heavily entangled with Overlays and At Junctions option groups so they got refactored too. In the process noticed that there were some duplicate Set.. functions in MaintenanceTab and PoliciesTab, so they got chainsawed.

Split option groups in to separate classes/files
Convert to CheckboxOption
Chainsaw numerous Set.. and On..Changed functions
Move bunch of methods from Options.cs to OptionsManager.cs
De-spam multiple calls to external methods during option loading
Ensure 'activated features' defaults are set properly

Fixes: #1423
Updates: #62

They were all entangled hence somewhat larger than expected commit.

Ensure mod options will always be set false if not validated.

Similar approach to CheckboxOption, but just a button.

- Move pathfind stats cb to overlays group - Split maint buttons in to two groups... - Tools group - Config group

krzychu124 · 2022-02-27T22:57:42Z

Do you have plans for Options checkbox that can use different class for storing/reading fields? I almost finished Allow despawn thingy but I'm thinking how to save/restore everything. Saving value for each vehicle type seems to be an overkill since we can read and store it as bitflags of ExtVehicleTypes.
Here is the list of available options, there are no overlapping so storing everything as single value will help with comparing.

! could use it like this instead of huge switch or chain of if...else

//MayDespawn if
|| Options.allowedDespawnVehicleTypes.IsFlagSet(extVehicle.vehicleType));

originalfoo · 2022-02-27T23:29:41Z

@krzychu124 We're limited to saving as byte currently = 8 bits.

EDIT: We could store it as a global option = much more flexible, eg. we could use int or whatever works best with xml.

krzychu124 · 2022-02-27T23:37:19Z

Yeah, global options might be a good idea to store this info

krzychu124 · 2022-02-28T01:26:55Z

We're limited to saving as byte currently = 8 bits.

Not if we get rid of ILegacySerializableOption interface from SerializableUIOptionBase and create another layer for byte options as a base for checkbox/slider options so I could create different type for GlobalConfig

On the other hand, we don't need load and save for globalconfig if we add event OnAfterValueSet or something like that (and set it to GlobalConfig.Save())

I'm not sure if you are going to change anything there but I can fix everything and create base for GlobalConfig options

originalfoo · 2022-02-28T02:15:28Z

The scope property has a Options.PersistTo.Global which I was planning to use (ie. when that's specified it will save to Main instance) but hand't got round to it yet.

Note that currently the checkboxes are defined as static fields on the group classes; I wasn't sure at what point global config gets loaded, and also what happens if it's reloaded or reset eg. in-game via the buttons on Maintenance tab? Like, how will that affect the instance reference?

krzychu124 · 2022-02-28T02:25:48Z

Note that currently the checkboxes are defined as static fields on the group classes; I wasn't sure at what point global config gets loaded, and also what happens if it's reloaded or reset eg. in-game via the buttons on Maintenance tab? Like, how will that affect the instance reference?

I already replaced static access with generic, reset/reload can be solver with observer. I'll experiment with that and try to add basic support for it. We can improve it later, since I already have working solution for "allow-despawn" feature but without persistence layer 😕

originalfoo · 2022-02-28T03:21:48Z

ready for review

krzychu124

Looks ok 👍

kvakvs

Looked through the changes, just minor naming

kvakvs · 2022-02-28T10:10:55Z

TLM/TLM/Manager/Impl/OptionsManager.cs

-            return SimulationAccuracy.MaxValue - value;
-        }
+        // See: OnAfterLoadData() and related methods
+        private static bool _needUpdateRoutingManager = false;


Confusing naming. Routing manager NEEDS update? We or someone else should update routing manager? Who will update?
Suggested naming: _updateRoutingManagerFlag | _routingManagerUpdateFlag, or _invalidateRoutingManager
Missing xmldoc. Don't send to read code instead of writing 1line explanation.

I have a hunch it might not even be necessary (ie. it can probably be completely removed) but need to check first... brb

RoutingManager.OnAfterLoadData() invokes RecalculateAll() so I don't need the _needUpdateRoutingManager stuff (during deserialisation, I can just ignore requests because post-deserialisation the RoutingManager is going to do full recalc anyway).

However I noticed that RequestFullRecalculation() is doing different stuff to RecalculateAll() (nothing to do with this PR) so that might merit some investigation at future date.

@kianzarrin Can you verify my assumptions in the comment above - that during deserialisation I don't need to trigger RequestFullRecalculation() becuase routing manager will subseuqently do a RecalculateAll()?

kvakvs · 2022-02-28T10:13:59Z

TLM/TLM/Manager/Impl/OptionsManager.cs

@@ -262,11 +303,11 @@ public bool MayPublishSegmentChanges()
                save[55] = OptionsMassEditTab.RoundAboutQuickFix_ParkingBanYieldR.Save();

                save[56] = PoliciesTab.NoDoubleCrossings.Save();
-                save[57] = PoliciesTab.DedicatedTurningLanes.Save();
+                save[57] = PoliciesTab_AtJunctionsGroup.DedicatedTurningLanes.Save();


Naming. Save is an action to write something somewhere. What Save here doing is Serilalization, maybe Serialize? if that's not a reserved name.

Yeah, that naming is from the original CheckboxOption before I started altering it. As it's just converting bool to byte maybe ToByte() would be better name?

I've added a task to #1356 to rename the methods.

The routing manager already does full recalc after deserialization so there's no need for options manager to queue that action

kianzarrin · 2022-03-02T14:31:27Z

@aubergine10 this commit ~~caused~~ has problem

https://images-ext-1.discordapp.net/external/PycS-lUNc7k7Hnsu2KBHSm78F5rrNH6GK1n5k5kjkmw/https/media.discordapp.net/attachments/545066721854357515/948480995840110602/unknown.png

kianzarrin · 2022-03-02T14:32:10Z

Actually it might be a commit before that but its in the PR

kianzarrin · 2022-03-02T14:32:38Z

red arrow is bad green arrow is good

originalfoo · 2022-03-02T18:28:34Z

thanks for heads up - investgating...

Refactor features, overlays, and junction options

ba05d5b

They were all entangled hence somewhat larger than expected commit.

originalfoo added Usability Make mod easier to use technical Tasks that need to be performed in order to improve quality and maintainability UI User interface updates Settings Road config, mod options, config xml labels Feb 18, 2022

originalfoo added this to the 11.6.5.1 milestone Feb 18, 2022

originalfoo self-assigned this Feb 18, 2022

originalfoo marked this pull request as draft February 18, 2022 02:10

originalfoo added 2 commits February 18, 2022 02:10

Merge branch 'master' into refactor-maintenance-features

65f4106

Simplify checkbox validators

dc9c3c4

Ensure mod options will always be set false if not validated.

originalfoo added Dependent This issue is blocked by another issue. DO NOT MERGE YET Don't merge this PR, even if approved, until further notice labels Feb 18, 2022

originalfoo mentioned this pull request Feb 18, 2022

RFI: Updates to mod options tabs #1356

Open

4 tasks

originalfoo added 2 commits February 18, 2022 10:43

Add ActionButton class

7071de3

Similar approach to CheckboxOption, but just a button.

Refactor remainder of maintenance tab

01dffb6

- Move pathfind stats cb to overlays group - Split maint buttons in to two groups... - Tools group - Config group

originalfoo mentioned this pull request Feb 19, 2022

Missing timed traffic lights and manual traffic lights buttons #1423

Closed

originalfoo added 4 commits February 19, 2022 02:48

Add button stuff from #1424

11a14ca

Update OptionButtonBase.cs

fd6b548

Merge branch 'master' into refactor-maintenance-features

129a022

Merge branch 'master' into refactor-maintenance-features

8c1bd24

originalfoo added Dependent This issue is blocked by another issue. and removed Dependent This issue is blocked by another issue. labels Feb 21, 2022

Merge branch 'master' into refactor-maintenance-features

38ca42f

Merge branch 'master' into refactor-maintenance-features

84c92cf

originalfoo requested review from kvakvs, DaEgi01 and krzychu124 February 28, 2022 03:20

originalfoo removed DO NOT MERGE YET Don't merge this PR, even if approved, until further notice Dependent This issue is blocked by another issue. labels Feb 28, 2022

originalfoo marked this pull request as ready for review February 28, 2022 03:21

krzychu124 approved these changes Feb 28, 2022

View reviewed changes

kvakvs approved these changes Feb 28, 2022

View reviewed changes

originalfoo added 4 commits February 28, 2022 14:29

Fix broken reference

1feedbf

Remove unnecessary routing manager update

8c47ed3

The routing manager already does full recalc after deserialization so there's no need for options manager to queue that action

Merge branch 'master' into refactor-maintenance-features

f517bff

Merge branch 'master' into refactor-maintenance-features

e3879ae

originalfoo merged commit b4d19a6 into master Mar 2, 2022

originalfoo deleted the refactor-maintenance-features branch March 2, 2022 02:25

originalfoo mentioned this pull request Mar 2, 2022

What's New in TM:PE 11.6.5.1 ? #1433

Merged

originalfoo mentioned this pull request Mar 2, 2022

Blank TM:PE toolbar buttons in Scenario Editor #1449

Closed

originalfoo mentioned this pull request Apr 4, 2022

Quick set-up for high priority roundabout does not work. (CTRL+SHIFT+CLICK) #1494

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor features, overlays, and junction option groups #1422

Refactor features, overlays, and junction option groups #1422

originalfoo commented Feb 18, 2022 •

edited

Loading

krzychu124 commented Feb 27, 2022 •

edited

Loading

originalfoo commented Feb 27, 2022 •

edited

Loading

krzychu124 commented Feb 27, 2022

krzychu124 commented Feb 28, 2022

originalfoo commented Feb 28, 2022 •

edited

Loading

krzychu124 commented Feb 28, 2022

originalfoo commented Feb 28, 2022

krzychu124 left a comment

kvakvs left a comment

kvakvs Feb 28, 2022

originalfoo Feb 28, 2022

originalfoo Feb 28, 2022

originalfoo Mar 1, 2022

kvakvs Feb 28, 2022

originalfoo Feb 28, 2022

originalfoo Mar 1, 2022

kianzarrin commented Mar 2, 2022 •

edited

Loading

kianzarrin commented Mar 2, 2022

kianzarrin commented Mar 2, 2022

originalfoo commented Mar 2, 2022

Refactor features, overlays, and junction option groups #1422

Refactor features, overlays, and junction option groups #1422

Conversation

originalfoo commented Feb 18, 2022 • edited Loading

krzychu124 commented Feb 27, 2022 • edited Loading

originalfoo commented Feb 27, 2022 • edited Loading

krzychu124 commented Feb 27, 2022

krzychu124 commented Feb 28, 2022

originalfoo commented Feb 28, 2022 • edited Loading

krzychu124 commented Feb 28, 2022

originalfoo commented Feb 28, 2022

krzychu124 left a comment

Choose a reason for hiding this comment

kvakvs left a comment

Choose a reason for hiding this comment

kvakvs Feb 28, 2022

Choose a reason for hiding this comment

originalfoo Feb 28, 2022

Choose a reason for hiding this comment

originalfoo Feb 28, 2022

Choose a reason for hiding this comment

originalfoo Mar 1, 2022

Choose a reason for hiding this comment

kvakvs Feb 28, 2022

Choose a reason for hiding this comment

originalfoo Feb 28, 2022

Choose a reason for hiding this comment

originalfoo Mar 1, 2022

Choose a reason for hiding this comment

kianzarrin commented Mar 2, 2022 • edited Loading

kianzarrin commented Mar 2, 2022

kianzarrin commented Mar 2, 2022

originalfoo commented Mar 2, 2022

originalfoo commented Feb 18, 2022 •

edited

Loading

krzychu124 commented Feb 27, 2022 •

edited

Loading

originalfoo commented Feb 27, 2022 •

edited

Loading

originalfoo commented Feb 28, 2022 •

edited

Loading

kianzarrin commented Mar 2, 2022 •

edited

Loading