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

#264: disable incompatible mod warning #286

Merged
merged 7 commits into from
Apr 20, 2019

Conversation

krzychu124
Copy link
Member

@krzychu124 krzychu124 commented Apr 17, 2019

Resolves #264 and resolves #284

Key features:

  • checkbox in options menu to turn off checking for incompatible mods
  • checkbox at the bottom of Incompatible mods detected dialog to turn off checking at next startup or mod enable (does the same as option above)
  • value is persisted in TMPE_GlobalConfig.xml

Minor bug to solve before merging: [Fixed in latest commit]
- when incompatible mods detected, Autorun at startup checkbox state in the Incompatible mods detected dialog is not syncronized with corresponding value displayed in Mod options menu, but value change is properly saved to config.

TODO:

@krzychu124 krzychu124 added enhancement Improve existing feature Usability Make mod easier to use LABS TM:PE LABS branch labels Apr 17, 2019
@krzychu124 krzychu124 self-assigned this Apr 17, 2019
@originalfoo
Copy link
Member

originalfoo commented Apr 17, 2019

Shall I add in the #284 filter to only check enabled mods?

@krzychu124
Copy link
Member Author

Updated description 😉

@originalfoo originalfoo added this to the 10.19 milestone Apr 17, 2019
@krzychu124 krzychu124 marked this pull request as ready for review April 17, 2019 19:25
@krzychu124
Copy link
Member Author

It looks like it is working as intended 😃

@originalfoo
Copy link
Member

originalfoo commented Apr 17, 2019

The text of these options is too similar IMO:

incompat

What about:

o Scan for known incompatible mods on startup
    o Ignore disabled mods
o Notify me if a new mod conflict is detected at any time

What do you think?

@krzychu124
Copy link
Member Author

Good point. I like it 😃

@originalfoo
Copy link
Member

originalfoo commented Apr 17, 2019

Or instead of:

o Notify me if a new mod conflict is detected at any time

Maybe:

o Notify me if there is an unexpected mod conflict

@originalfoo
Copy link
Member

auto

Maybe change to:

o Scan for known incompatible mods on startup

This way it uses same locale string as in mod options, and also more closely ties together the checkbox on the pop-up with the checkbox in mod options?

@originalfoo
Copy link
Member

The options all seem to work as intended btw. Both on start up and also when enabling/disabling the mod.

@krzychu124
Copy link
Member Author

I will make all changes tomorrow 😉

@originalfoo
Copy link
Member

Missing update to https://github.com/krzychu124/Cities-Skylines-Traffic-Manager-President-Edition/blob/master/TLM/TLM/Resources/lang_template.txt which is used as basis of new localisations.

@krzychu124
Copy link
Member Author

krzychu124 commented Apr 18, 2019

Good catch...
I think something is broken because I can't see that file in project files view 😕 Alphabetically sorted:

Zrzut ekranu (156)

Anyway I'll update it 😃

@originalfoo
Copy link
Member

originalfoo commented Apr 18, 2019

Well, it shouldn't be included in the workshop package, which is why it's not in the solution.

I wonder if it's even needed, as we could just tell people to use the lang.txt as template instead?

I used the lang_template.txt in the localisation guide as that's what @VictorPhilipp had in the original wiki documentation for localisation. It's essentially a clone of lang.txt as far as I can tell.

@krzychu124
Copy link
Member Author

I think that for translation better would be lang.txt because using just template you can't translate tutorial keys.

Copy link
Contributor

@Emphasia Emphasia left a comment

Choose a reason for hiding this comment

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

lang_zh.txt updated.

TLM/TLM/Resources/lang_zh.txt Outdated Show resolved Hide resolved
TLM/TLM/Resources/lang_zh.txt Outdated Show resolved Hide resolved
TLM/TLM/Resources/lang_zh.txt Outdated Show resolved Hide resolved
TLM/TLM/Resources/lang_zh.txt Outdated Show resolved Hide resolved
@originalfoo
Copy link
Member

@krzychu124 I've updated the localisation page in wiki to state lang.txt should be used as template.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

LGTM!

@originalfoo
Copy link
Member

@FireController1847 Can you confirm the skipping disabled mods thing works as you expect?

@krzychu124
Copy link
Member Author

I've updated translations from @Emphasia review and included those from PR in @VictorPhilipp repo as well

@originalfoo
Copy link
Member

LGTM! Ok to merge?

Copy link
Collaborator

@FireController1847 FireController1847 left a comment

Choose a reason for hiding this comment

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

Works as intended. Would be nice to ignore disabled by default but that's more of my personal preference haha. Skipping the 3-day wait time due to strong requests to merge approved by me!

@originalfoo
Copy link
Member

I agree with @FireController1847 , having the 'ignore disabled' be active by default would be better.

Good catch @FireController1847 !

@krzychu124
Copy link
Member Author

Changed 😉

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

LGTM!

@originalfoo originalfoo merged commit 60528d8 into master Apr 20, 2019
@krzychu124 krzychu124 deleted the 264-disable-incompatible-mod-warning branch July 27, 2019 22:13
@originalfoo originalfoo added the COMPATIBILITY Mod (in)compatibility / checker label Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMPATIBILITY Mod (in)compatibility / checker enhancement Improve existing feature LABS TM:PE LABS branch Usability Make mod easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatible Detector Should Not Warn When Not Enabled Disable incompatible mod warning option
4 participants