Page MenuHomeWildfire Games

Rmgen new type constraint DensityConstraint
Needs RevisionPublic

Authored by nani on Sep 22 2018, 2:13 AM.

Details

Reviewers
FeXoR
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Add new rmgen DensityConstraint. Gives the ability to define how probable is to set something in a given position.

Useful for creating gradual tree density changes.

Test Plan

Used and tested in map Riverway: https://code.wildfiregames.com/D1632

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

nani created this revision.Sep 22 2018, 2:13 AM
nani created this object with visibility "nani".
nani edited the test plan for this revision. (Show Details)Sep 22 2018, 5:24 PM
nani changed the visibility from "nani" to "Public (No Login Required)".Sep 22 2018, 5:55 PM
Freagarach added a reviewer: Restricted Owners Package.Mar 24 2020, 7:15 PM
FeXoR requested changes to this revision.May 8 2020, 3:31 PM
FeXoR added a subscriber: FeXoR.

This constraint is not deterministic since the random number is rolled in .allows.
That means if you use this to paint trees on an area and do this over and over again you will get increasing densities though that was nowhere specified by the user.
This is not at all how a constraint should work IMO :p

I also find the name of this constraint quite misleading for I would have guessed it calculates the density of something like the type of objects divided by the given area those objects are in (the density of those objects) and allows/denies weather it's within a density range given to this constraint.
But instead it returns some random values and takes into account steps and height.

This revision now requires changes to proceed.May 8 2020, 3:31 PM
badosu added a subscriber: badosu.EditedMay 8 2020, 3:37 PM

This constraint is not deterministic since the random number is rolled in .allows.
That means if you use this to paint trees on an area and do this over and over again you will get increasing densities though that was nowhere specified by the user.

I mean, if you paint two times in a row certainly you shouldn't expect the state to be stored from the previous paint job? One could arguably store the state on a singleton but I think this constraint achieves the purpose (I could imagine its application in a myriad of circumstances as-is).

I also find the name of this constraint quite misleading for I would have guessed it calculates the density of something like the type of objects divided by the given area those objects are in (the density of those objects) and allows/denies weather it's within a density range given to this constraint.

If you think about probability density and that the end result (for a high enough sample size) achieves the purpose, I am not opposed to it.

<strikethrough>All above said though, I don't like the idea of pulling library functions that are not used, to avoid code rot. If brought with a map (or changes to a map) that makes use of it then I'm all for it.</strikethrough>
Ah, I see the new map patch (riverway), I'll test it 👍

FeXoR added a comment.EditedMay 8 2020, 4:11 PM

"I mean, if you paint two times in a row certainly you shouldn't expect the state to be stored from the previous paint job?" I agree. But a constraint returns a boolean value and should return the same when checked multiple times in a row.
"this constraint achieves the purpose" What is it's purpose? For smoothly changing tree density dependent on distance to area and/or height I suggest to use a painter rather than a constraint ;)
And there are maps that have smoothly changing tree densities (without steps btw.) in the codebase already like e.g. deep_forest.js that are generated without this constraint (or a placer for it).

badosu added a comment.May 8 2020, 5:02 PM

For smoothly changing tree density dependent on distance to area and/or height I suggest to use a painter rather than a constraint ;)

That makes sense