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: Don't try to rename OWNER_DEITY signs in-game #9716
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment Ian"t great - it explains "what", which the code already does. Comments should explain "why", if needed
It's very possible to edit signs of other companies. For good or not but currently, you just highjack the sign when you edit it. The only ownership check CmdRenameSign does is for OWNER_DEITY which is entirely different from the check in this PR:
|
59cbf96
to
3519172
Compare
That's...odd. I've amended the check and PR title to reflect the real itch being scratched by this PR, which is trying to edit a sign we're not allowed to change. Hopefully the comment better explains the "why" now, too. |
Maybe move sign ownership check to a separate function? To reduce the chance of anyone making the same mistake again. |
It's a pretty specific usecase, but something like |
3519172
to
ef6b670
Compare
|
ef6b670
to
a32f310
Compare
Motivation / Problem
Many city builder Game Scripts use a sign below the town name to show the current growth rate. Players can click on this sign which brings up the GUI to edit the sign, but they are actually blocked from renaming the sign. The same goes for signs created by other players.
Why show the rename GUI if we're not allowed to do anything with it?
Description
If we're not allowed to rename a sign, don't even open the rename GUI.
The exception is if we're in Scenario Editor, which can open savegames by changing the file extension, and needs to be allowed to edit anything including signs.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.