Following rP19247 the diplomacy window is updated on each tick, so we can remove the old updateDiplomacy function which now only updates the player data, and rename updateDiplomacyPanel to updateDiplomacy for consistency with other windows.
Details
Details
- Reviewers
elexis - Commits
- rP19257: cleanup of the updateDiplomacy functions
Check that the diplomacy window still works as wanted
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/424/ for more details.
Comment Actions
The removal of the function is good, but not too fond of the rename, as it's not clear from the name whether it changes some data variable (like the function with that name used to) or whether it refreshes the GUI (what it now does). updateDiplomacyDialog might work if thats better than panel
Comment Actions
The renaming to updateDiplomacy is to make it consistent with the openDiplomacy and closeDiplomacy functions. I see no problem to have that consistency by keeping updateDiplomacyPanel and moving to openDiplomacyPanel and closeDiplomacyPanel (i don't like dialog here). But then the same move will be needed (still to keep consistent) for the other panels as Trade.
Comment Actions
Take care before committing, there is an occurance of updateDiplomacy(); in gui/common/functions_global_object.js that updated the ceasefire counter in the diplomacy dialog, but it can be safely removed as the dialog is now updated onSimUpdate regardless.
Going to move that code asap.
Okay I see your point and accept the rename.