Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1541 - Fixing lack of iron chunks in cave #2389

Closed
wants to merge 9 commits into from
Closed

1541 - Fixing lack of iron chunks in cave #2389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1bc70a6
Changed iron density in cave to match small/big chunk ratios of Meso
Maxonovien Jul 6, 2021
6d225df
Does one attempt per spawner up to max instead of max per spawner to …
Maxonovien Jul 6, 2021
6bb73ab
Code optimisation, comments, cleaning
Maxonovien Jul 6, 2021
244e92b
Initialized attemptsPerSpawnType
Maxonovien Jul 6, 2021
32e67e7
Added code explanation
Maxonovien Jul 6, 2021
59848d4
Styling
Maxonovien Jul 7, 2021
e8c0c56
Fix Styling
Maxonovien Jul 7, 2021
18925d6
Fixed styling
Maxonovien Jul 7, 2021
9df6863
Replaced min and max by mathf.clamp
Maxonovien Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion simulation_parameters/microbe_stage/biomes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@
"convexShapePath": "res://assets/models/Iron4.shape"
}
],
"density": 0.0001,
"density": 0.001,
"dissolves": true,
"radius": 1,
"chunkScale": 1,
Expand Down
30 changes: 25 additions & 5 deletions src/microbe_stage/SpawnSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ public class SpawnSystem
/// </summary>
private int estimateEntityCount;

private Dictionary<Spawner, int> attemptsPerSpawnType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be better to have the attempts counter in the Spawner class itself? Or is there somthing prohibiting this I don't see?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are right, the spawner-dependent part could be stored in Spawner indeed for memory saving.

However, given that I think I'm finally succeeding with a more efficient rework with PR #2397 (which also solves this spawn order thing), I think I'll discontinue this one for a bit, and only resume if the other PR actually does not work / takes longer and a hotfix is needed.


public SpawnSystem(Node root)
{
worldRoot = root;
attemptsPerSpawnType = new Dictionary<Spawner, int>();
}

// Needs no params constructor for loading saves?
Expand Down Expand Up @@ -232,7 +235,18 @@ private void SpawnEntities(Vector3 playerPosition, Vector3 playerRotation, int e

int spawned = 0;

attemptsPerSpawnType.Clear();
int maxAttempts = -1;
foreach (var spawnType in spawnTypes)
{
int attempts = Mathf.Clamp(spawnType.SpawnFrequency * 2, 1, maxTriesPerSpawner);
attemptsPerSpawnType[spawnType] = attempts;

if (attempts > maxAttempts)
maxAttempts = attempts;
}

for (int i = 0; i < maxAttempts; i++)
{
/*
To actually spawn a given entity for a given attempt, two
Expand All @@ -247,14 +261,20 @@ To allow more than one entity of each type to spawn per
spawn cycle, the SpawnSystem attempts to spawn each given
entity multiple times depending on the spawnFrequency.
numAttempts stores how many times the SpawnSystem attempts
to spawn the given entity.
to spawn the given entity. Furthermore, each attempt gives a spawn chance
for each spawner (up to their type limit) to still favor diversity,
instead of one spawner taking up all the free spawning slots.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the issue that the spawners were making so much of one thing that they were reaching the maximum of living entities, which kept iron from spawning in the cave? From testing this change clearly works, but I don't understand why.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could certainly be a thing. For a while now, I've wanted per entity spawn caps (that are more reasonable, for example I think if the 1000 entity limit fills up with cells the game will lag, a lot).

Copy link
Contributor Author

@Maxonovien Maxonovien Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was not even the total cap, but the per-frame cap (toxyns were taking all slots (compounds would still go first as they spawned, I guess), that's why decreasing their density led to iron spawning)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per-frame? As in the spawner has been adding 500 toxyns per frame? I certainly haven't observed that in game...

Copy link
Contributor Author

@Maxonovien Maxonovien Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the per-frame limit is yet another thing, and is much smaller (2, I think). So at each frame, 2 entities would be spawned if that did not go over total cap aswell (which makes sense, both to not have long spawning frame and to be able to spawn new things to adapt - although it could be raised shortly after new patch spawn).

Yet, given that toxins had such a high spawning rate, they always spawned up to the frame spawn limit (they roughly had ~100 chances out of 500 (max cap) so 20% chances, for 50 spawn trials or so). And thus, spawners later in the list were not tested in this frame, and the list would be reset at the next frame.

I guess that another fix could have been to save the remaining spawner list for next frame to cycle it all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a while now, I've wanted per entity spawn caps

Capping toxins to 100, and having a separate cap for iron and marine snow chunks, is what I meant in my comment. This doesn't have anything to do with per-frame caps, which also probably could be problematic if the toxins take up all the per-frame spawns... this is addressed by: #2138 (which is an incomplete PR to change the spawn system) which uses a round robin approach to spawn from a randomized item list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed a per-entity cap is a fully independant question, related to the total cap which is not the reason for the issues fixed here (but I agree this could prove useful overall).

This PR was really about identifying their cause and fixing them within the current system as they really harm game experience in patches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also going to take a look at the incomplete spawn change PR though, see if I feel like taking it up.

*/
int numAttempts = Math.Min(Math.Max(spawnType.SpawnFrequency * 2, 1),
maxTriesPerSpawner);

for (int i = 0; i < numAttempts; i++)
foreach (var spawnType in spawnTypes)
{
if (random.Next(0, numAttempts + 1) < spawnType.SpawnFrequency)
int attempts = attemptsPerSpawnType[spawnType];

// Already did max attempts for this type
if (i > attempts)
continue;

if (random.Next(0, attempts + 1) < spawnType.SpawnFrequency)
{
/*
First condition passed. Choose a location for the entity.
Expand Down