Page MenuHomeWildfire Games

Consistent starting resources and customizable population cap for scenarios
AbandonedPublic

Authored by Wundersam on Dec 25 2018, 5:39 PM.

Details

Reviewers
smiley
Trac Tickets
#5371
Summary

Changes will allow the population cap to be set when a scenario type map is selected. Before, in order to set the population cap, one would have to change to a non-scenario map, set the population cap and change back.
Will also hide starting resources from the game description of scenarios because they'd be ignore in InitGame.js:67

if (settings.mapType !== "scenario" && settings.StartingResources)
{
	let resourceCounts = cmpPlayer.GetResourceCounts();
	let newResourceCounts = {};
	for (let resouces in resourceCounts)
		newResourceCounts[resouces] = settings.StartingResources;
	cmpPlayer.SetResourceCounts(newResourceCounts);
}

I also added myself to the list of programmers as this is my first patch.

Test Plan

Manual testing was performed in the relevant game screen.
This revealed unexpected behaviour unrelated to the changes where game settings are reset when clicking on "random" in the map type dropdown; This will be adressed in another changeset.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6517
Build 10779: arc lint + arc unit

Event Timeline

Wundersam created this revision.Dec 25 2018, 5:39 PM
Owners added a subscriber: Restricted Owners Package.Dec 25 2018, 5:39 PM
Wundersam edited the summary of this revision. (Show Details)Dec 25 2018, 5:41 PM
elexis added a subscriber: elexis.Dec 25 2018, 6:46 PM

As far as I know, scenario maps are supposed to be fixed in settings, skirmish maps changeable.

Well, they weren't fixed, it just wasn't possible to change them when scenario was selected but the last used values where used which made arbitrary settings possible.
I chose to allow setting population cap because that wasn't further prevented in the code and the default 300 population cap might be a performance barrier for some people with weaker hardware.

smiley requested changes to this revision.Dec 26 2018, 12:08 PM
smiley added a subscriber: smiley.

Goes against the design. The bug where settings persisted should be fixed though.
Requesting changes. Feel free to remove the status though.

This revision now requires changes to proceed.Dec 26 2018, 12:08 PM

Well, they weren't fixed, it just wasn't possible to change them when scenario was selected

= settings fixed for scenario, as intended as far as I can see from the code history

but the last used values where used which made arbitrary settings possible.

That's the classic persist-match-settings bug, that equally occurs when selecting a different map where the previous map had specified a default but the new map didn't.
This is a systematic error and needs a redesign of the implementation, which will become more feasible after having cleaned the code further, I have planned a branch for #5322 for this file.

binaries/data/mods/public/gui/common/gamedescription.js
404

There are starting resources for scenario maps, so the sentence is a bit wrong.
Ultimately there should not be two ways to specify starting resources, or otherwise the two ways to define resources (resource per player vs. all resources of all players at a time) #4504.

smiley resigned from this revision.Jan 15 2019, 9:03 AM

I would also like to apologise for the misunderstanding earlier. It wasn’t meant to say “your patch sucks”. It was meant to point you at the underlying bug without conflicting with the game design.

This revision now requires review to proceed.Dec 18 2019, 5:49 PM
elexis abandoned this revision.Dec 24 2019, 6:12 AM

See inline comments. Thanks for the food for thought.

binaries/data/mods/public/gui/common/gamedescription.js
404

It should not become consistent with the simulation code since the simulation code is broken.
Currently it copies the per-player values to the g_GameAttributes value but then ignores it for the skirmish map.
But Atlas still provides a way to specify the per-player values, so map authors will think it has use for skirmish maps.
And there is a considerable use case - why not have per-player population limits and starting resoures on skirmish (and random) maps.
D2483 changes both places to consume the map-specified per-player limit and the global limit for players that don't have a specific value.

binaries/data/mods/public/gui/gamesetup/gamesetup.js
606

(This change is against the current scenario design (current scneario design = everything fixed, skirmish = most settings free))

There are many tickets asking for random and skirmish maps to become more restricted.
This patch goes the other way and proposes to make scenario maps more unrestricted.

Consequentially there is no more difference between Skirmish and Scenario maps and the two maptypes may be merged (wrote something about it in D2483), but not so soon.