Opened 8 years ago
Last modified 20 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 )
These gamesetup options new in alpha 21 should become settable in atlas.
Attachments (2)
Change History (9)
by , 7 years ago
Attachment: | 4256_atlas_settings_v1.patch added |
---|
comment:1 by , 7 years ago
Keywords: | rfc patch added |
---|---|
Milestone: | Backlog → Work 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 , 7 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 , 7 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()
oftest_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 supportsU32
values while atlas UI only supportsi32
as far as I know, so everything above2^16
would be lost by the GUI. Unfortunately we don't have atlas GUI tests afaik.
by , 7 years ago
Attachment: | 4256_atlas_settings_v1.1.patch added |
---|
Fix issues pointed out by elexis, and use a numerical input element for the wonder victory timeout.
comment:4 by , 7 years ago
Keywords: | rfc removed |
---|
The patch has been ported to Phabricator. https://code.wildfiregames.com/D95
comment:5 by , 7 years ago
Keywords: | patch removed |
---|---|
Summary: | [PATCH] Atlas - Wonder victory timeout and last man standing checkbox → Wonder victory timeout and last man standing checkbox |
comment:6 by , 3 years ago
Description: | modified (diff) |
---|---|
Owner: | set to |
Patch: | → Phab:D95 |
comment:7 by , 20 months ago
Owner: | removed |
---|
Backlogging due to lack of progress, lack of review, or users going MIA.
Add wonder victory timeout, last man standing option, fix a TODO, rename ID_MapTeams to ID_MapLockTeams.