CivMC / Civ

Monorepo for development of and running a Civ Server.
MIT License
4 stars 11 forks source link

Always spawn ore spawns if a block is available #378

Closed okx-code closed 3 months ago

okx-code commented 3 months ago

Currently, when spawning ores, HiddenOre will select a random adjacent block to the block broken up to three times and replace it with the ore block if it is not air. This has the problem that there is a small chance that all three times, the random face will be air. This should be fixed to consider all faces, or even a 3x3x3 cube around the block broken.

Fixing this would probably slightly increase ore spawns by a couple percent, depending on how exactly they are mined.

Solitaire7 commented 3 months ago

Currently, when spawning ores, HiddenOre will select a random adjacent block to the block broken up to three times and replace it with the ore block if it is not air. This has the problem that there is a small chance that all three times, the random face will be air. This should be fixed to consider all faces, or even a 3x3x3 cube around the block broken.

Fixing this would probably slightly increase ore spawns by a couple percent, depending on how exactly they are mined.

There's a setting in the config for hidden ore that drops the attempted spawn as an item on the floor if there are no viable surfaces found for the spawn attempt if I remember correctly. This could be another way to solve the issue of there not being a viable surface for the ore to spawn on when you break a block

ProgrammerDan commented 3 months ago

Huh, someone added face spawning.

Code under consideration: https://github.com/CivMC/Civ/blob/main/plugins/hiddenore-paper/src/main/java/com/github/devotedmc/hiddenore/listeners/BlockBreakListener.java#L343

The number of attempts is controlled by configuration, then further enlarged by number of visible faces.

In general, I'm quite certain this is not how I would have approached it, but from my read of code if you're seeing a lot of failure cases, switch back to random bloom with better configuration parameters. Devoted used the random bloom exclusively, and based on my statistic modelling generated ore at the desired rate. Or, increase the transformattemptmultiplier to a larger number (6 perhaps) and you'll sample additional times.

Alternatively, if the bloom method is too misunderstood or no-one is interested in some help configuring it, definitely implement the cuboid approach instead of the current faces approach. The faces approach definitely means any spawn configuration that wants to generate more than 3 ores will likely fail, whereas the bloom approach was specifically engineered to avoid this problem entirely by considering an area around the broken block and sampling a configurable number of times while increasing the search radius as it goes. This is not fully accurate, but you can imagine the bloom method does the following, all with just math and no hardcoding: faces are inferentially sampled, then the full 3x3x3, then the faces of that 3x3x3, then the full 5x5x5, if configured to do so, as the radius of consideration increases as the number of failed spawn attempts increases, as configured.

okx-code commented 3 months ago

The random bloom approach you described does sound much nicer, is it something we can just configure or does it need to be added in the code? @ProgrammerDan

awoo-civ commented 3 months ago

The faces approach definitely means any spawn configuration that wants to generate more than 3 ores will likely fail

This seems accurate based on my experience mining. I've never had 4 diamond ores spawn.

ProgrammerDan commented 3 months ago

@okx-code It's there, just have to turn off the use of the faces approach.

okx-code commented 3 months ago

@ProgrammerDan well, I'm looking at the code and I don't see how it's done, are you sure it's still there?

ProgrammerDan commented 3 months ago

A little more meat -- looks like transform_attempt_multiplier is a root config option, by default set to 3, you can increase that. Then, in each vein config, look for forceVisibleTransform -- if set to "true", that's what turns on forced faces. Turn it off to switch back to radial, random incremental search bloom.

ProgrammerDan commented 3 months ago

@okx-code the one thing I'd meant to mention is if it still seems like either the bloom is too tight and ores are still not spawning at the desired rates (check statistics, measure against % of occurrence against configured, measure size of spawn to be sure it fits your median expected -- complex when analyzing against tool modifiers, but possible with some careful queries, if you're tracking this) -- consider implementing the ToDo here: https://github.com/CivMC/Civ/blob/main/plugins/hiddenore-paper/src/main/java/com/github/devotedmc/hiddenore/listeners/BlockBreakListener.java#L352 -- in this case, u0 is the "0.5" static value, which pushes the initial edge of the spawn attempts into the "faces" of the point-of-break. As the attempts increase (cAttempt), this value grows by the cube root. I had hoped to add a multiplier -- uA in the comment -- to allow it to more rapidly sweep outwards for a far more radial arc kind of bloom. The cube root with sufficient attempts slows radial growth enough that clustering still felt tight on testing and review, so I never bothered adding either parameter, but they could be easily added.