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.

Sign up for GitHub

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

Jump to bottom

Revise warning that was confusing users #1347

Merged
merged 10 commits into from
Feb 2, 2022
Merged

Revise warning that was confusing users #1347

merged 10 commits into from
Feb 2, 2022

Conversation

originalfoo
Copy link
Member

@originalfoo originalfoo commented Feb 2, 2022 •
edited

This PR removes revises the warning dlg about vehicles being despawned due to pathfinder update. Many users thought it was an error (it's using warning prompt UI as we don't have any alternative at present), so just removed the warning dlg for now updated how it works (see #1347 (comment) ).

@originalfoo
Remove warning that was confusing users
aadd755 
Many thought it was an error (it's using warning prompt UI as we don't have any alternative at present), so just removed the warning dlg for now.
@originalfoo originalfoo added high priority Affects lots of users Usability Make mod easier to use labels Feb 2, 2022
@originalfoo originalfoo added this to the 11.6.4-hotfix-5 milestone Feb 2, 2022
@originalfoo originalfoo self-assigned this Feb 2, 2022
@krzychu124
Copy link
Member

@aubergine10 check current mode before despawning. No need to show/trigger it when users is starting new game or there was no TM:PE save data yet

@originalfoo
Copy link
Member Author

I was thinking of that but how do I know if they just started a new city?

Also, most people will be loading an existing city so they'll see the message and think it's an error. I had a look for "info"-styled CO UI but couldn't find anything yet.

@originalfoo
Copy link
Member Author

Updated - message is shown again, but only if there were actually vehicles to despan - this will prevenrt dlg appearing on new games, or cities that don't have that vehicle type. Also revised message to hopefully make it more obvious that it's not an error, but a fix for a past error.

@originalfoo originalfoo changed the title Remove warning that was confusing users Revise warning that was confusing users Feb 2, 2022
@krzychu124
Copy link
Member

I was thinking of that but how do I know if they just started a new city?

Also, most people will be loading an existing city so they'll see the message and think it's an error. I had a look for "info"-styled CO UI but couldn't find anything yet.

OnLevelLoaded has parameter of load mode and I think the same info could be read via SimulationManager metadata or it's stored somewhere on the SimulationManager instance

@originalfoo
Copy link
Member Author

Now checks Mode property to make sure it's not NewGame or NewGameFromScenario

(I assume Mode is the correct one to check vs. LoadMode ?)

@originalfoo originalfoo mentioned this pull request Feb 2, 2022
Closed
9 tasks
originalfoo added a commit that referenced this pull request Feb 2, 2022
@originalfoo
What's New in TM:PE 11.6.4.5 ?
48a34a3 
This release makes some refinements to error checking/logging, and refines the pathfinder edition check/warning.

- [Meta] TM:PE 11.6.4-hotfix-5
- [Meta] Updates to error checking/logging, and refine pathfinder edition checks
- [Updated] Catch and log errors in savegame options save/load #1345 (aubergine18)
- [Updated] Pathfinder edition check refinements #1347 (aubergine18)
- [Updated] Reduce severity of some normal log messages #1348 #1350 (aubergine18)
@originalfoo originalfoo mentioned this pull request Feb 2, 2022
Merged
@krzychu124
Copy link
Member

Updated - message is shown again, but only if there were actually vehicles to despan - this will prevenrt dlg appearing on new games, or cities that don't have that vehicle type. Also revised message to hopefully make it more obvious that it's not an error, but a fix for a past error.

Have you tried MessageBoxPanel instead of ExceptionPanel? It should have Ok button and nothing else.

@krzychu124 krzychu124 self-requested a review February 2, 2022 23:11
@krzychu124
Copy link
Member

Oh shit, I've never seen that vanilla dialog before😂 @aubergine10 is it good enough?

image

@originalfoo
Copy link
Member Author

@krzychu124 Sorry for delay in replying... yes that is significantly better!

@originalfoo
Copy link
Member Author

Spotted a typo though - will send a commit shortly

@originalfoo
Copy link
Member Author

done :)

@originalfoo
Copy link
Member Author

Maybe add that dlg to the Prompt class as an Info() method?

@krzychu124 krzychu124 merged commit 5853243 into master Feb 2, 2022
@krzychu124 krzychu124 deleted the remove-path-despawn-warning branch February 2, 2022 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krzychu124 krzychu124 krzychu124 approved these changes

Assignees

@originalfoo originalfoo

Labels
high priority Affects lots of users Usability Make mod easier to use
Projects
None yet
Milestone
11.6.4-hotfix-5
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@originalfoo @krzychu124