JamieNyanchi
commented
1 year ago Updated to add in a few more changes since I submitted the PR. As a note, this update doesn't make any changes to the parts that @Infixo commented on though. So those will still need to be resolved. As for the new changes. I updated the original post with them, but I'll list the additions here too:
-
Properly add shadow to non-valid tiles when in District/Building Placement mode without having to click twice.
-
Show/Hide growth tile based on whether it's a valid spot when clicking through various Districts/Buildings. Previously, it's visibility would only be determined for the first district selected. If you selected another district/building without exiting the placement mode, the growth tile's visibility would not change. Now it will update for each district/building selected.
-
Do not add shadow to tiles within cities when not in District/Building Placement mode. An example of this happening before would be when a city can no longer purchase any more tiles (after growing to include all tiles within 3 of the city center). The citizen management view will then add shadows to tiles that cannot be worked. This is due to how the ShowPurchases function works in the base game. It will add city tiles to its list if any tile can be purchased. This would not happen if tiles cannot be purchased. After this PR, no city tiles will have a shadow on them.
An alternative solution to 6 would be to always add shadow to tiles that cannot be worked. I have included the code for this in the PR, but it is commented out.
Here is how cities will look with the changes active in the PR:
Here is how the cities will look with the alternative solution:
Notice the difference in how tiles are highlighted. I'm not sure which would be preferred so I picked the one I think looks better (the first one), but I'm open to feedback about which would be better.
Infixo
This pull request implements the following seven items:
Refactor city view closing code to simplify closing the view with LuaEvents. For example, CityPanelOverview no longer needs to check the interface mode to decide what to do when the close button is clicked. This change also makes better use of the CQUI_OnInterfaceModeChanged function in the CityPanel file.
Ensure the city view is closed when the city is deselected. Sometimes the city view can stay up even when the city is deselected. One case I noticed this happening in was the OnCityLoyaltyChanged function being called while being selected on a city. I would be selected on a city in-between turns, and it would deselect the city at the beginning of the next turn, but leave the city view up. This isn't game breaking, but causes bad UI. For instance, clicking through the tabs on the overview panel will show some blank tabs and also throw out errors in the Lua.log file With these changes, the city view is always closed if the city is deselected for any reason.
Exit out of the District/Building placement mode when a new turn begins. The SelectedUnit file from the base game sets the lens to Default in its OnLocalPlayerTurnBegin function. This clears out the District/Building placement view, but doesn't exit it, which looks confusing. Now the District/Building placement mode is exited as well.
Properly add shadow to non-valid tiles when in District/Building Placement mode without having to click twice.
Show/Hide growth tile based on whether it's a valid spot when clicking through various Districts/Buildings. Previously, it's visibility would only be determined for the first district selected. If you selected another district/building without exiting the placement mode, the growth tile's visibility would not change. Now it will update for each district/building selected.
Do not add shadow to tiles within cities when not in District/Building Placement mode. An example of this happening before would be when a city can no longer purchase any more tiles (after growing to include all tiles within 3 of the city center). The citizen management view will then add shadows to tiles that cannot be worked. This is due to how the ShowPurchases function works in the base game. It will add city tiles to its list if any tile can be purchased. This would not happen if tiles cannot be purchased. After this PR, no city tiles will have a shadow on them.
In the CityOverviewPanel, there is a bar for the city growth. However, it is unused for some reason. It is redundant information since it's displayed elsewhere on the CityPanel, but I feel it should either be used or removed. I chose to use it here.
I want to make note of a couple things I noticed while making these changes:
When implementing change 2, I noticed that the OnCityLoyaltyChanged function (which can be found in files such as CityPanel_Expansion1) was being called in CQUI, but not in the base game (for the same turns on the same game). This function is called by the engine though, so why is this happening? In CQUI, it deselects the city I'm selected on even though the city's loyalty didn't change?
When implementing change 3, I originally wanted to modify SelectedUnit to only set the lens to Default if not in the District/Building placement mode. However, I had difficulty doing this because of how SelectedUnit_Expansion2 is added to the game. I think the problem is that in the Expansion2.modinfo file, "SelectedUnit_Expansion2" is not listed under the Import Files section. This seems to make it impossible to include the file in a modded SelectedUnit file? As a result, I think I'd either have to completely replace the base game's SelectedUnit file, or copy/paste all of the code from SelectedUnit_Expansion2 into a new file if I wanted to modify it? As shown in this PR though, I ultimately decided to not touch it and resolve this small bug with a different approach.
These were just a couple things I thought were interesting.
Anyway, I'm more than happy to make any changes or provide additional info if requested!