Page MenuHomeWildfire Games

Consistent starting resources and customizable population cap for scenarios
Needs RevisionPublic

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.