BloodyMods / ExNihiloCreatio

Ex Nihilo with progression!
MIT License
25 stars 22 forks source link

[Suggestion] Make AutoSifter Maximum Rotational Power Configurable #282

Open ghost opened 4 years ago

ghost commented 4 years ago

Hey there !

I've been digging throught the code to try and figure out why it felt like my auto sifter setup didn't seem to get any faster.

found out that the rotational power is limited to 100.

image https://github.com/BloodyMods/ExNihiloCreatio/blob/75f60692a4257a62ba9f6831866e456d8526ec9d/src/main/java/exnihilocreatio/tiles/TileAutoSifter.java#L65

I suspect it's designed that way since going over 100 means having 200+ water wheels and chunk loading could become a problem after that. But since there's a lot of modpacks including various ways to chunkload a region, I was wondering if it wouldn't be better to make that value configurable ?

Thanks !

SirLyle commented 4 years ago

What that line is actually doing is: once 100 units (or -100 units) of rotational power has built up it performs a single sieving step. Increasing that number would actually slow down your sieves.

The part that controls how fast you build up rotational units is the perTickRotation calculated by calcEffectivePerTickRotation.

ghost commented 4 years ago

Then there might be a strange issue hiding somewhere. After having placed over 200 waterwheels, I couldn't see any improvement on the sieving speed.

I suspect this might be due to a chunk loading or chunk entity processing issue. I'll check once I have access to my computer.

SirLyle commented 4 years ago

It still does act as a cap just in a more roundabout way; each sieving step is discrete. So once you hit 1 sieving step per update tick (which is actually every 10 ticks) it won't go any faster.

ghost commented 4 years ago

Done a lot of testing, and I realized half way that I was doing this for absolutely nothing.

Re read the code and now I'm confident that I understand really well.

You're right, increasing 100 would slow down the sieving process. Although I also found that the sieving algoritm is flawed.

The thing is, TileAutoSifter is an implementation of ITickable, meaning that the update method is called every tick (But you know that). The issue is that the storedRotationalPower value is updated every tick by adding the value of perTickRotation, and if it exceeds 100, 100 is removed from it.

image

If perTickRotation value is 101, here what happens:

Since the baseProgress value is 0.1f, and you need to reach 1 for a sieving operation to complete: The maximum speed of the AutoSieve (if the mesh is not enchanted and player does not have haste) is 0.5 seconds, or 10 ticks.

ghost commented 4 years ago

I thought of a possible way of solving this cap, but I suppose this cap could also be a design decision

ghost commented 4 years ago

Currently, the amount of ticks required to call the doAutoSieve function follows this mathematical equation:

ticksRequired = 100 / perTickRotation;

You could add the result of that equation to the TileSieve doSieving function, and add it to the progress calculation.

the progress would then become:

progress = baseProgress / result +efficiencyScale efficiencyValue (1f + hasteScale * hasteValue)

ghost commented 4 years ago

That would allow for a more scallable autoSieve, a more configurable sieving speed and voiding the risk of storedRotationalPower to reach float.MaxFloat (which is likely to never happen, but the risk is there)

Edit: and would also be a change of maybe 5 modified lines and 5 removed

ghost commented 4 years ago

that would be the change required (requires tweaking to match java's syntax and rules)

// TileSieve.java
public void doSieving(EntityPlayer player, boolean automatedSieving, float autoSieveStep) {

    // ...

    float step;

    // not sure how to handle undefined parameters in java ...
    if (!autoSieveStep && autoSieveStep < 1){
        step = ModConfig.sieve.baseProgress;
    }
    else{
        step = ModConfig.sieve.baseProgress / autoSieveStep;
    }

    progress += (step + ModConfig.sieve.enchantments.efficiencyScaleFactor * efficiency)
                      * (1f + ModConfig.sieve.enchantments.hasteScaleFactor * haste);

    // ... 
}

// TileAutoSieve.java
@Override
public void update() {
    // ...

    // if (Math.abs(storedRotationalPower) > 100 && toSift != null) {
    //     storedRotationalPower += storedRotationalPower > 0 ? -100 : 100;
    //     if (!world.isRemote) {
    //         doAutoSieving(toSift);
    //     }
    // }  
                          // config for the value of 100 ...
    float autoSieveStep = ModConfig.sieve.requiredRPforSieveStep / perTickRotation;

    if (!world.isRemote){
        doAutoSieving(toSift, autoSieveStep);
    }

    // ...
}

private void doAutoSieving(TileSieve[][] sieveMap, float autoSieveStep) {
    for (TileSieve[] tileSieves : sieveMap) {
        for (TileSieve tileSieve : tileSieves) {
            if (tileSieve != null) {
                if (!itemHandlerAutoSifter.getStackInSlot(0).isEmpty() && tileSieve.addBlock(itemHandlerAutoSifter.getStackInSlot(0))) {
                    itemHandlerAutoSifter.getStackInSlot(0).shrink(1);
                }
                tileSieve.doSieving(null, true, autoSieveStep);
            }
        }
    }
}