Opened 6 years ago

Last modified 5 months ago

#4256 new defect

Wonder victory timeout and last man standing checkbox

Reported by: elexis Owned by:
Priority: Should Have Milestone: Work In Progress
Component: Atlas editor Keywords:
Cc: Patch: Phab:D95

Description (last modified by Silier)

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

Attachments (2)

4256_atlas_settings_v1.patch (11.8 KB ) - added by Sandarac 6 years ago.
Add wonder victory timeout, last man standing option, fix a TODO, rename ID_MapTeams to ID_MapLockTeams.
4256_atlas_settings_v1.1.patch (5.5 KB ) - added by Sandarac 6 years ago.
Fix issues pointed out by elexis, and use a numerical input element for the wonder victory timeout.

Download all attachments as: .zip

Change History (9)

by Sandarac, 6 years ago

Add wonder victory timeout, last man standing option, fix a TODO, rename ID_MapTeams to ID_MapLockTeams.

comment:1 by Sandarac, 6 years ago

Keywords: rfc patch added
Milestone: BacklogWork In Progress
Summary: Atlas - Wonder victory timeout and last man standing checkbox[PATCH] Atlas - Wonder victory timeout and last man standing checkbox

comment:2 by Sandarac, 6 years ago

The last man standing checkbox will be disabled and set to false if lock teams is set to true, like in gamesetup.js. The wonder victory timeout option will be disabled when a non-wonder victory condition is selected (it might be nice to use a slider for the timeout option).

comment:3 by elexis, 6 years ago

About the feature:

  • I'd be happy about numerical input elements like for starting resources (there is also a ticket for numerical inputs for resources and wonder victory time in the gamesetup, #4381). bb suggested to use a slider. It's just my personal opinion, so I might be wrong.
  • L149 could use a range loop, see wiki:Coding_Conventions#Misc
  • (Merge conflicts with the atlas 2 UI ticket / github rewrite (#3414). It is heavily outdated, but if updated and rebased, your patch will be rewritten in some way.)
  • Keeping the rfc keyword as the patch looks like it works and improves the code. (Didn't run it though).
  • Sentences start with a capital letter.

About the tests:

  • Don't like duplicate json files. A similar discussion occured in #2951.

In case we keep the _test copy, so that the test can be executed without a public mod, the json file could be reduced a bit, to make it more transparent that we don't require an exact copy of the original and so it's less to read (while still covering a number of values, probably even a wider range than in the original). The test json is also completely independent from the public json. Someone can completely rewrite one json format and the tests still succeed. It were useful if the tests would fail. It could try to (additionally?) load the json file from the public mod and repeat the test (there was code cited launching the public mod cited in #2951)

  • Your test only checks whether some strings are included in the JSON file, bad practice. The test should verify that the JSON file is actually a valid JSON string, that the numbers occur in the right place etc.. Compare with test_json() of test_ScriptInterface.h. (Contrary to what the name suggests, ReadJSON only opens the file and parses it as UTF8 text).
  • The tests are supposed to check for edge cases, so why not add 2^16. The answer would be that the test doesn't actually test anything except that a file load works. The simulation supports U32 values while atlas UI only supports i32 as far as I know, so everything above 2^16 would be lost by the GUI. Unfortunately we don't have atlas GUI tests afaik.

by Sandarac, 6 years ago

Fix issues pointed out by elexis, and use a numerical input element for the wonder victory timeout.

comment:4 by Sandarac, 6 years ago

Keywords: rfc removed

The patch has been ported to Phabricator. https://code.wildfiregames.com/D95

comment:5 by Stan`, 6 years ago

Keywords: patch removed
Summary: [PATCH] Atlas - Wonder victory timeout and last man standing checkboxWonder victory timeout and last man standing checkbox

comment:6 by Silier, 2 years ago

Description: modified (diff)
Owner: set to Sandarac
Patch: Phab:D95

comment:7 by Stan`, 5 months ago

Owner: Sandarac removed

Backlogging due to lack of progress, lack of review, or users going MIA.

Note: See TracTickets for help on using tickets.