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

Fix #8922: Crash when selling last member of shared order group while window was open #8923

Closed
wants to merge 1 commit into from

Conversation

LordAro
Copy link
Member

@LordAro LordAro commented Apr 1, 2021

Motivation / Problem

#8922
Vehicle GUI window (while showing shared orders) could become empty while open (selling all vehicles in group)

Description

Close the window when shared order window's vehicle list becomes empty
Window was doing updates of internal state in OnPaint rather than OnInvalidateData which struck me as a bit weird, so moved stuff

Played around with it a bit (though not extensively), window still updates when it should, and doesn't crash

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LordAro
Copy link
Member Author

LordAro commented Apr 1, 2021

@btzy You may be interested in taking a look

@LordAro LordAro added the backport requested This PR should be backport to current release (RC / stable) label Apr 1, 2021
@btzy
Copy link
Contributor

btzy commented Apr 1, 2021

This doesn't seem to work on my Windows computer... with this commit, toggling the "Group By" dropdown (i.e. step 2 of #8922) only changes the number on the left of each row, but doesn't actually group anything. Without this commit, toggling the "Group By" dropdown works.

(The bug in #8922 can be reproduced on my computer though.)

@btzy
Copy link
Contributor

btzy commented Apr 1, 2021

I think this is not the right fix for the bug. The root cause seems to be due to the fact that the user can now have a "Shared orders of 1 vehicle" window opened. Previously, it was not possible to have such a situation because the shared orders window closes automatically when the user unshares the shared orders of 2 vehicles.

@btzy
Copy link
Contributor

btzy commented Apr 1, 2021

I made PR #8926, that prevents opening a shared order list of just one vehicle.

@LordAro
Copy link
Member Author

LordAro commented Apr 2, 2021

Closed in favour of #8926

@LordAro LordAro closed this Apr 2, 2021
@LordAro LordAro removed the backport requested This PR should be backport to current release (RC / stable) label Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants