Page MenuHomeWildfire Games

Add a "new" checkbox to the map settings tab
ClosedPublic

Authored by trompetin17 on Jun 25 2019, 4:52 AM.

Details

Reviewers
elexis
Commits
rP22397: #5445
Trac Tickets
#5445
Test Plan

We need to test in windows, linux and OSX,

You must see the third option in Map Settings, and you will be able to change xml with keyword "new"

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8070
Build 13137: Vulcan BuildJenkins
Build 13136: arc lint + arc unit

Event Timeline

trompetin17 created this revision.Jun 25 2019, 4:52 AM
Owners added a subscriber: Restricted Owners Package.Jun 25 2019, 4:52 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1809/display/redirect

elexis added a subscriber: elexis.Jun 25 2019, 4:59 AM

You may want to add the trigger keyword, since there is also a filter for that in the gamesetup.

var g_MapFilters = [
        {
                "id": "default",
                "name": translateWithContext("map filter", "Default"),
                "tooltip": translateWithContext("map filter", "All maps except naval and demo maps."),
                "filter": mapKeywords => mapKeywords.every(keyword => ["naval", "demo", "hidden"].indexOf(keyword) == -1),
                "Default": true
        },
        {
                "id": "naval",
                "name": translate("Naval Maps"),
                "tooltip": translateWithContext("map filter", "Maps where ships are needed to reach the enemy."),
                "filter": mapKeywords => mapKeywords.indexOf("naval") != -1
        },
        {
                "id": "demo",
                "name": translate("Demo Maps"),
                "tooltip": translateWithContext("map filter", "These maps are not playable but for demonstration purposes only."),
                "filter": mapKeywords => mapKeywords.indexOf("demo") != -1
        },
        {
                "id": "new",
                "name": translate("New Maps"),
                "tooltip": translateWithContext("map filter", "Maps that are brand new in this release of the game."),
                "filter": mapKeywords => mapKeywords.indexOf("new") != -1
        },
        {
                "id": "trigger",
                "name": translate("Trigger Maps"),
                "tooltip": translateWithContext("map filter", "Maps that come with scripted events and potentially spawn enemy units."),
                "filter": mapKeywords => mapKeywords.indexOf("trigger") != -1
        },
        {
                "id": "all",
                "name": translate("All Maps"),
                "tooltip": translateWithContext("map filter", "Every map of the chosen maptype."),
                "filter": mapKeywords => true
        },
];

(Notice 1024x768 is the supposed minimum resolution, and the one used by at least one of our devs on a laptop, so might have to use 2 lines for the checkboxes.)

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

The default mapfilter does not care if the new keyword is present.
Selecting the new filter in the gamesetup means that only new maps are shown.

The map should have the new keyword if it should appear in the filter of new maps.

320

loop or macro?

trompetin17 updated this revision to Diff 8602.Jun 25 2019, 5:18 AM

Add "trigger" Keyword in Map settings
Change Checkbox to be 2 by line

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1810/display/redirect

Looks good, I got some compile warnings however (gcc 9.1.0):

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

The comments are still a bit wrong for the new / trigger keyword.

How about

"If checked, the map will appear in the list of new maps"
"If checked, the map will appear in the list of maps with trigger scripts"

trompetin17 updated this revision to Diff 8603.Jun 25 2019, 5:26 AM

Fix messages description

trompetin17 marked an inline comment as done.Jun 25 2019, 5:27 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1811/display/redirect

elexis accepted this revision.EditedJun 25 2019, 6:03 AM
  • Task of the ticket is reasonable.
  • Task of the ticket was extended to entail the Trigger keywords.
  • Review remarks were addressed.
  • Didn't get more compiler warnings than I already had before.
  • Tested, works in accordance with the gamesetup filters.
  • Thanks for the patch!
  • Fix that map -> maps typo before the commit and perhaps 5 -> 15 for better visual layout

Edit: Code also reads like it has to work platform independently.

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

One can change the third argument here (4, 5, 5) to (4, 5, 15) to have the checkboxes more spread out. Otherwise I didnt lookup how the checkboxes can be centered.

193

list of new map -> list of new maps

194

maps

330

technically, redundant semicolons, but I guess consistent with the rest

This revision is now accepted and ready to land.Jun 25 2019, 6:03 AM
trompetin17 updated this revision to Diff 8604.Jun 25 2019, 6:13 AM

Change VIsual Setting

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1812/display/redirect

trompetin17 updated this revision to Diff 8605.Jun 25 2019, 6:15 AM

Fixed Typo

elexis accepted this revision.Jun 25 2019, 6:16 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1813/display/redirect

trompetin17 closed this revision.Jun 25 2019, 7:12 AM