HomeWildfire Games

Add random map Wild Lake. Patch by FeXoR. Reviewed by elexis and Imarok.
Concern RaisedrP19704

Description

Add random map Wild Lake. Patch by FeXoR. Reviewed by elexis and Imarok.

Differential Revision: https://code.wildfiregames.com/D548

Details

Auditors
elexis
Committed
FeXoRJun 1 2017, 10:46 AM
Differential Revision
D548: Wild Lake random map script
Parents
rP19703: STUN + XMPP ICE implementation.
Branches
Unknown
Tags
Unknown
Build Status
Buildable 2080
Build 3364: Post-Commit BuildJenkins

Event Timeline

elexis raised a concern with this commit.Oct 3 2017, 11:51 PM
elexis added a subscriber: elexis.
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/maps/random/wild_lake.js
743

The 70 lines from below here are a copy & paste from caledonian meadows.

This commit now has outstanding concerns.Oct 3 2017, 11:51 PM
elexis accepted this commit.Nov 6 2017, 2:52 PM
All concerns with this commit have now been addressed.Nov 6 2017, 2:52 PM
elexis raised a concern with this commit.Dec 11 2018, 3:09 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/maps/random/wild_lake.js
395

This mixes actor and non-actor entities, but actor entities can be placed on the 3 tiles at the map border whereas non-actor entities may not be placed there, leading to either unreachable trees and other code complaining about unreachable entities or the 3 border tiles not being placed on the map border.

Reported at https://wildfiregames.com/forum/index.php?/topic/24908-grumpy-gurkens-recenst-ramblings/&page=3&tab=comments#comment-365577

The solution is to either split this into two arrays each or detecting if the handled entity is an actor.

400

(It's not clear what HS means. Looking at the code reveals a slope difference. Perhaps the SlopeConstraint could be used for consistency. Also it would be less fragmented if the height intervals are contained in this object too, for example like pyrennean sierra. It is just a HeightConstraint, so these two constraints might ormight not be combined here.)

This commit now has outstanding concerns.Dec 11 2018, 3:09 PM
FeXoR marked 2 inline comments as done.Dec 11 2018, 4:48 PM
FeXoR added inline comments.
/ps/trunk/binaries/data/mods/public/maps/random/wild_lake.js
395

"[...] or detecting if the handled entity is an actor."
I agree with this (Though I'd say "a" solution BTW, not "the" solution). But not here but when it's placed.

(So rather open a ticket than raising a concern about an old commit would IMO be the way to go so we can talk about that in terms of line numbers or make use of Phabricator's inline comments ;) )

The line where the check could happen should contain: if (actor)
...to be extended by an "is actor" check.
(The variable "actor" can be an entity or an actor)

400

HS stands for "High Slope".
Yes, the SlopeConstraint could be used but that would be number of height limits times slower.
The height limits (that make the height intervals) could go here, but since calculated the values of that properties would just be Undefined (n more lines for no gain but confusion IMO).
The height dependent placement in this map is not realized in the "constraint" way of the random map libraries (Would need number of height limits times more time).

So your proposal would make this 14 times slower, need 7 more lines where nothing is set ... so I'm not for it ;p

Also the distance to gaia in nomad mode must be increased, otherwise spawnkill. Actually it's probably better to not place gaia in nomad mode at all, as the placeable map area might be too few if the distance is increased, or if just a player runs into a gaia group.