Page MenuHomeWildfire Games
Differential D95

Atlas - Wonder Victory Timeout and Last Man Standing Checkbox
Changes PlannedPublic
Actions

Authored by Sandarac on Jan 25 2017, 5:12 AM.

Details

Reviewers
vladislavbelov
elexis
Trac Tickets
#4256
Summary

These gamesetup options new in Alpha 21 should become settable in atlas.

The wonder victory time has been added as a numerical input element. The relationship between the last man standing checkbox and the lock teams checkbox is handled in the same way as in gamesetup.js (the last man standing option is disabled and set to false if lock teams is set to true).

This patch also rewords the confusingly named ID_MapTeams to ID_MapLockTeams and reorders the enum in the order the GUI elements are displayed under Map Settings. Game type options are loaded by filename in the victory_conditions directory (to fix a TODO).

I know that this conflicts with trompetin17's Atlas UI branch (as noted in the trac ticket), but that repository doesn't seem to have been updated in around a year.

Test Plan

When Atlas is opened, a number can be entered in the GUI element for the wonder victory time if the game type has been set to "wonder". The GUI element will be disabled otherwise. If the "Last Man Standing" option is selected, and then the "Lock Teams" option is selected afterwards, the "Last Man Standing" option will be deselected and become disabled.

A scenario map can be saved with a set wonder victory duration, and then loaded in the 0 A.D. gamesetup, which will show the wonder victory duration.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 1055
Build 1662: Vulcan BuildJenkins

Event Timeline

Sandarac created this revision.Jan 25 2017, 5:12 AM
Vulcan added a subscriber: Vulcan.Jan 25 2017, 5:56 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/244/ for more details.

Harbormaster completed remote builds in B356: Diff 338.Jan 25 2017, 5:56 AM
elexis added subscribers: trompetin17, elexis.Jan 28 2017, 12:01 AM

Reads good at first sight.

I'm a bit reluctant to change atlas though while #3414 / https://github.com/0ad/0ad/compare/master...trompetin17:atlasUI2 is still relatively easy to rebase.
That branch is one year behind, yet there were only 11 commits affecting atlas: https://github.com/0ad/0ad/commits/master/source/tools/atlas
4 of them were whitespace changes.
The other commits seem quite easy to rebase as well. The regicide change seems to be the only one that needs some actual code treatment.
This proposed patch will also have to be rewritten once that is in.

So someone should really fork the branch by @trompetin17, rebase it and get it committable, so that we can start changing things again without those side effects (Combining victory conditions will also affect atlas.)

elexis added a reviewer: elexis.Apr 9 2017, 2:07 AM
Sandarac updated this revision to Diff 1163.Apr 9 2017, 8:03 AM

Update after the change made in rP19345.

Vulcan added a comment.Apr 9 2017, 8:54 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/717/ for more details.

Sandarac updated this revision to Diff 1164.Apr 9 2017, 9:11 AM

Use wxTextValidator(wxFILTER_DIGITS) for the Victory Duration and the random seed numerical inputs.

Harbormaster completed remote builds in B1054: Diff 1163.Apr 9 2017, 9:12 AM
Sandarac planned changes to this revision.Apr 9 2017, 9:20 AM

Personally, I think that it would still be nice to merge AtlasUI2 eventually, regardless of the changes made in rP19391.

When it comes to this diff, allowing any non-negative number for "Victory Duration" results in warnings in gamesetup.js if the number is not present in victory_times.json (for example, using the number "24"). I haven't looked into it much yet, but this means there will likely need to be changes made in updateGUIObjects() in gamesetup.js.

Vulcan added a comment.Apr 9 2017, 10:03 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/718/ for more details.

Harbormaster completed remote builds in B1055: Diff 1164.Apr 9 2017, 10:03 AM
elexis edited edge metadata.Apr 9 2017, 2:33 PM

Well, it is planned to replace those dropdown number choices with actual numerical text input choices (ticket somewhere), should also be used for ceasefire. Starting resources, population capacity and mapsize could have numerical text input + dropdown combined some day (predefined choices but allowing for even more freedom).

The input validation added in the most recent change is good:
http://docs.wxwidgets.org/trunk/valtext_8h.html#aa02d29254d60e0c81f17696c9cecbd07a724e6e8a40da5bdef9ec2389da3ccebf

Sandarac added a comment.EditedMay 7 2017, 2:55 AM

Now with the recent changes in gamesetup.js, no warnings are thrown when a map specifies a Victory Duration value that is not in victory_times.json, but when looking under "More Options", the Victory Duration value is displayed as "Unknown" (even though it is shown correctly in the game-description, and the value is also passed correctly to the simulation).

It seems reasonable to adapt the gui code to handle values that are not hardcoded in JSON files.

vladislavbelov added a subscriber: vladislavbelov.Aug 12 2017, 2:46 PM
vladislavbelov requested changes to this revision.Aug 12 2017, 2:55 PM
vladislavbelov added inline comments.
source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
44

What's the reason of the reordering? If alphabetic order, why ID_Map* aren't sorted?

149

It should be:

for (const wxString& gameType : gameTypes)
165

wxFILTER_DIGITS doesn't present in the wxWidgets 2.8, so it's invalid.

http://docs.wxwidgets.org/2.8/wx_wxtextvalidator.html#wxtextvalidator

elexis resigned from this revision.Dec 12 2017, 8:17 PM
Stan added a subscriber: kenny.Sep 18 2019, 8:04 AM

Revision Contents
Changeset List

PathSize
source/
tools/
atlas/
AtlasUI/
ScenarioEditor/
Sections/
Map/
67 lines

Diff 1164

View Options

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp

Loading...
Wildfire Games · Phabricator