Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve mod options lifecycle in editors #1425

Merged
merged 15 commits into from
Mar 4, 2022

Conversation

originalfoo
Copy link
Member

@originalfoo originalfoo commented Feb 18, 2022

Compiled mod for testing: TMPE.zip

TM:PE mod options should not be persisted while in editors, particularly the map/scenario editor (which are essentially savegames; any user starting new city from them would get unexpected options set = confuses users).

This PR makes the following changes:

  • Mod options are never persisted in editors
    • Note: Other data such as road customisations is unaffected by this change
  • When starting a new game, options will always be reset to default
  • When loading a savegame, options are loaded normally
  • If no data to deserialize, default options will be applied

Additionally:

  • TTL tools can now be used in the map/scenario editor

Fixes: #1423
Updates: #1452
Updates: #1449
Updates: #959

Notably: Don't save while in editor (particularly map editor).
@originalfoo originalfoo added Settings Road config, mod options, config xml Asset Editor Issue related to TM:PE support in content editors labels Feb 18, 2022
@originalfoo originalfoo added this to the 11.6.5.1 milestone Feb 18, 2022
@originalfoo originalfoo self-assigned this Feb 18, 2022
@originalfoo
Copy link
Member Author

originalfoo commented Feb 18, 2022

@krzychu124 Is the CI artifact thing still disabled?

@krzychu124
Copy link
Member

How does that fix the issue? In asset editor that function is not used at all IIRC

@krzychu124
Copy link
Member

@krzychu124 Is the CI artifact thing still disabled?

Enabled and rerun builds, we will see if works

@originalfoo
Copy link
Member Author

@krzychu124 Is the CI artifact thing still disabled?

Enabled and rerun builds, we will see if works

Yup, working again :)

@originalfoo
Copy link
Member Author

How does that fix the issue? In asset editor that function is not used at all IIRC

The asset editor has auto-save interval in map editor. IIRC that will be triggering any mods to save their stuff in to the map (which is essentially a savegame file tagged "Map" instead of "Savegame").

@originalfoo
Copy link
Member Author

originalfoo commented Feb 19, 2022

If is saving TM:PE options to map in the editor, then a better fix would be to check for PlayMode right at the outset in TMPELifecycle:

public override void OnSaveData() => if (PlayMode) Save();

Note: I have no clue what effect that would have on whatever it is that TM:PE does to store road customisations in the editors.

@krzychu124
Copy link
Member

I'll check the save loop later to see what is going on

@originalfoo
Copy link
Member Author

I suspect the primary cause is the way we currently define defaults in the option fields themselves which means they can be overwritten by previous in-game or in-editor session when doing subsequent load in the same app session.

#1423 (comment)

@originalfoo originalfoo removed this from the 11.6.5.1 milestone Feb 19, 2022
@originalfoo originalfoo marked this pull request as draft February 20, 2022 00:45
@krzychu124
Copy link
Member

Is it ready for review? After looking how it works it seem that should fix the issue

@originalfoo
Copy link
Member Author

Was it actually saving options/etc to map in Map Editor?

@krzychu124
Copy link
Member

Map created in the map editor is like regular save game but running in special mode to limit interactions. Once created, new games created from it becomes detached from the source so any later updates to original map won't affect games that has been already created.
I've not tested it yet, but knowing that it makes sense.

@originalfoo
Copy link
Member Author

Should I move the PlayMode check to the TMPELivecycle.OnSaveData() as mentioned in comment above?

I have no clue how road customisation data gets saved in map/asset editor (ie. stuff we do want storing in the map/asset file) so not sure if it will have unintentional effects.

@originalfoo
Copy link
Member Author

originalfoo commented Feb 27, 2022

Also... should I add something that checks NewGame or NewGameFromScenario and if true prevent loading options (and other?) data? That would help solve any issues relating to maps that are already out there containing stored options/settings.

@originalfoo originalfoo added this to the 11.6.5.1 milestone Feb 27, 2022
@krzychu124
Copy link
Member

Should I move the PlayMode check to the TMPELivecycle.OnSaveData() as mentioned in comment above?

I have no clue how road customisation data gets saved in map/asset editor (ie. stuff we do want storing in the map/asset file) so not sure if it will have unintentional effects.

If you mean if (PlayMode) Save(); -> that will completely break saving other things in the map editor (arrows, lane connections etc.)

should I add something that checks NewGame or NewGameFromScenario

yes, that might help if the user subscribed to already published maps on workshop

(and other?)

Which other? AFAIK Timed traffic lights could be enabled in the map editor since created "map" is in 99% a regular savegame, but still disabled in asset editor because asset segments/nodes needs to be remapped after creation (asset editor segment/node ids are just the template to mirror settings) and we don't have mapper for TTL

For `OptionsManager`:

- Save only occurs if `PlayMode`
- Load will reset to defaults if not loading a savegame
@originalfoo originalfoo marked this pull request as ready for review February 27, 2022 23:21
@originalfoo
Copy link
Member Author

originalfoo commented Feb 27, 2022

Tweaked how it works - see OP for details.

EDIT: Ready for review.

@originalfoo
Copy link
Member Author

@krzychu124 any further comments on this?

@kianzarrin
Copy link
Collaborator

I do allow NC options to be stored in map mode so that the when the user loads a map the roads wouldn't change slopes.

@krzychu124
Copy link
Member

@krzychu124 any further comments on this?

I'm going to test it when I clear my github notification list :)

@originalfoo
Copy link
Member Author

Need some additional tweaks as per #1452 (comment)

brb

@originalfoo originalfoo added the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Mar 3, 2022
@originalfoo originalfoo changed the title Only save options in PlayMode Improve mod options lifecycle in editors Mar 3, 2022
@originalfoo
Copy link
Member Author

Just pushed a commit which:

  • Allows TTL tool in map/scenario editor
    • ...essentially a save game, so TTL customisations should carry across to new games
  • Only force option reset to default when starting new games (from map or scenario).

@originalfoo originalfoo removed the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Mar 3, 2022
@originalfoo
Copy link
Member Author

Waiting for more review prior to merge.

@originalfoo originalfoo added the TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc label Mar 3, 2022
@krzychu124
Copy link
Member

Wiki needs an update because features listed in the table below are enabled by default 😉

image

As of PR, tested scenario, looks like it nicely saves and loads tmpe feature settings, options are not loaded unless I already saved wrong settings with previous version of the mod. Still need to test map save and restore of TTL, just in case if you haven't already.

@originalfoo
Copy link
Member Author

Wiki needs an update

Done :)

Still need to test map save and restore of TTL, just in case if you haven't already.

I've only tested the scenario editor, I've not yet tested map editor.

@kianzarrin
Copy link
Collaborator

it does fix the icons in new game
image

@krzychu124
Copy link
Member

it does fix the icons in new game image

Yeah, I already merged in atlas fix from #1451 :)
We didn't test saving/loading settings in Map editor and that's probably the last thing that needs to be tested before merging this.

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working in map editor and scenario editor, loads and saves all tm:pe features, mod options are not saved 👍 Works as expected in the Asset/intersection editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Asset Editor Issue related to TM:PE support in content editors Settings Road config, mod options, config xml TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing timed traffic lights and manual traffic lights buttons
4 participants