Implemented throttleable Large turbine

[galyfray]

What:
Implemented feature proposed in #1548 : make the player able to throttle Large turbines like large boilers.

How solved:
I extracted code from the Large boiler to an abstract class ThrottleableMultiblockController and then make FueledMultiblockController extend it. For compatibility sake two constructors has been made one with the canThrottle argument and one without considering the argument as false by default. This way any code based on the FueledMultiblockController should work the same without any issue.
Created a ThrottleableRecipeLogic class to handle machine throttle and made LargeTurbineWorkableHandler extending it and changed some methods from private to protected in the FuelRecipeLogic as I was needing to override some part of it.

I added some information about fuel consumption rate making setting the correct throttle easy.
I also added a new shortcut on setting throttle : pressing control multiply the change by 10 leading with :

• shift + click => +/-1
• click => +/-5
• ctrl + shift + click => +/-10
• ctrl + click => +/- 50

Outcome:
Implement : #1548

Additional info:
New large turbine's controller GUI :

The boiler's controller GUI didn't change :

Possible compatibility issue:
Any machines that extends MetaTileEntityLargeTurbine will have to either support throttle or add this.canThrottle = false after calling the super constructor to disable the throttle ability.

[warjort]

Any machines that extends MetaTileEntityLargeTurbine will have to either support throttle or add this.canThrottle = false after calling the super constructor to disable the throttle ability.

This is not just a possibility:
https://github.com/Gregicality/gregicality/blob/master/src/main/java/gregicadditions/machines/multi/override/MetaTileEntityLargeTurbine.java
Although addons that use gregtech.common are not officially supported we should not deliberately break them.

IMO this is an anti-pattern

-    private int recipeDurationLeft;
-    private long recipeOutputVoltage;
+    protected int recipeDurationLeft;
+   protected long recipeOutputVoltage;

-   private void tryAcquireNewRecipe() {
+   protected void tryAcquireNewRecipe() {

  @Override
    protected int tryAcquireNewRecipe(FluidStack fluidStack) {
        int amount = (int) (super.tryAcquireNewRecipe(fluidStack) * ((ThrottleableMultiblockController) this.metaTileEntity).getThrottleMultiplier());
        if (amount > 0) {
            this.recipeOutputVoltage *= ((ThrottleableMultiblockController) this.metaTileEntity).getThrottleEfficiency();
        }
        return amount;
    }

Breaking encapsulation to do policy overrides usually leads to unmaintainable code.
This is especially true for something as important as "recipeDurationLeft" which is key to how FuelRecipeLogic works with implicit assumptions about how it changes.

Policy should be injected into the place where the state exists otherwise there is no way to reason about whether the object is in a consistent state or what implications any future change may have.

This could be done with more focused overridable methods that don't change the internal state in subclasses.

I think this throttle functionality would be better implemented in FuelRecipeLogic with optional enabling by subclasses.
When not enabled it should just behave as a 100% throttle (meaning it would be fully backwards compatible for existing subclasses).

It could be something as simple as the following methods to override which would also enable other more general uses.

public class FuelRecipeLogic {

/**
 * @return the multiplier applied to fuel consumption
 */
protected float getFuelConsumptionMultiplier() {
    return 1.0;
}

/**
 * @return the multiplier applied to recipe duration
 */
protected float getRecipeDurationMultiplier() {
    return 1.0;
}

/**
 * @return the multiplier applied to voltage 
 */
protected float getEfficiency() {
    return 1.0;
}

I also find it hard to read your changes when large amounts are unnecessary code reformatting so I haven't looked at the rest of the changes.

[galyfray]

I also find it hard to read your changes when large amounts are unnecessary code reformatting so I haven't looked at the rest of the changes.

first of all sorry for that I added a shortcut each time I save a file idea reformat it according to the standard present in the GTCE project and I didn't pay attention they was this much of reformat.

Breaking encapsulation to do policy overrides usually leads to unmaintainable code.
This is especially true for something as important as "recipeDurationLeft" which is key to how FuelRecipeLogic works with implicit assumptions about how it changes.

You're absolutely right I know breaking encapsulation is often a bad idea I just didn't found better way to do the job at the moment.

I think this throttle functionality would be better implemented in FuelRecipeLogic with optional enabling by subclasses.
When not enabled it should just behave as a 100% throttle (meaning it would be fully backwards compatible for existing subclasses).

This could be a good idea but other multiblock already use the throttle functionality stored in the meta tile entity then it feels a much proper way to implement it in one class each multiblock that want to use it can extends. I we wanted to implement it in the recipe logic it would be needed to be in a lower level to avoid duplicated code.
On an other hand my current implementation behave with the same advantages than the one you propose a default throttle to 100% with an activation decided by subclass : in any of those two cases the throttle would get activated in a large turbine related class and would then impact addons in a similar way.

IMO this is an anti-pattern
I'm not a native speaker of english please avoid things like IMO I can't understand what it says :) (and feel free to tell me if you found some errors) anyway I also don't know what is an anti-pattern.

This is not just a possibility:
https://github.com/Gregicality/gregicality/blob/master/src/main/java/gregicadditions/machines/multi/override/MetaTileEntityLargeTurbine.java
Although addons that use gregtech.common are not officially supported we should not deliberately break them.
I take a look to the code and my patch should not cause a breaking bug maybe some useless throttle button on the GUI . Still the bug could be fixed in one line which is the best I could have done so far.

Moreover the best I've done so far is not the best way you pointed some big problems in my code and think I have found a solution to some of them. The FuelRecipeLogic problem is a big one as said I don't this storing the throttle in the recipe logic is a good way to avoid breaking large boiler related code but still the current implementation hasn't been made in a way that was supposed to be extended. This leads me to think a good solutio would be to revert my changes to the current FuelRecipeLogic class and mark it as deprecated, making a new one in the API packages doing the same things but in a way that could be easly extended without making tricky bad code like I did. I think I can reduce the amount of code that would be required to implement the throttle behavior to any class extending the Large turbine but to my mind this would still require some work from the add-on maker ...

[warjort]

IMO = In my opinion

https://en.wikipedia.org/wiki/Anti-pattern
Something people do all the time but is usually wrong.

[galyfray]

thank you for those precision in deed this looks like a not so good idea but some idea like that was already in the Large turbine code see this and this

If anyone want to share some opinion/ advice or things I should take in account about remaking the FuelRecipeLogic I'll be happy as it will help me providing the best solution for anyone

[galyfray]

just realize that fuel recipe logic is all ready in the api gonna implement as proposed with some slight modification

[LAGIdiot]

I mostly agree with @warjort regarding this. We should try not break our addons even if they are using common part of mod. Regarding antipattern and best practices we should not add any more technical debt and try to leave edited files in better way then they were before (of course going on cleaning spree is not desired).

As you mentioned that you have idea how to rework then please do and I or someone else will do review on new version.

[galyfray]

What have changed :

• added a bunch a new utility function to the FuelRecipeLogic allowing to control various consumption and efficiency
• made the large turbines use those function to control energy production instead of overloading startRecipe and getRecipeOutputVoltage
• removed the ThrottleableRecipeLogic class

On the Gregicality side :
As I cleaned up the way the energy is produced in our controller the gregicality one saddly doesn't produces energy anymore, the quick fix is in startRecipe to return (getMaxVoltage() * getEnergyEfficiency()) and code from getRecipeOutputVoltage will have to be mostly moved to getEnergyEfficiency to fix the outputted energy showed on the GUI and the outputted energy being wrong.

Those changes break a little of addons but nothing to difficult to fix.
I changed the way the Large Turbine produce energy to cleanup it's behavior has most of the code from getRecipeOutputVoltage has been moved to getEnergyEfficiency I thought it was way cleaner to do this way as in any case by moving some code to handle efficiency this would break energy output of the turbine or making the code stranger not really maintainable.

[warjort]

The canProduceEnergy() and canConsumeFuel() don't appear to be used?
Is this left over from a previous iteration?

[warjort]

    @Override
    protected float getFuelConsumptionMultiplier() {
        return (float) this.largeTurbine.getThrottleMultiplier();
    }

I think perhaps these new methods should be using doubles instead of floats.
The ThrottleableMultiblockController uses doubles because that is what the LargeBoiler uses.

It probably won't make much difference but it will keep it consistent and avoid unnecessary casting like that above.

[galyfray]

The canProduceEnergy() and canConsumeFuel() don't appear to be used?
Is this left over from a previous iteration?

they are used in the update method

[warjort]

The ThrottableMultiblockController looks to me like it would be better implemented as a kind of throttle widget rather than a subclass? Similar to how things like filters fit into covers.

I know this is not how the LargeBoiler does it but it is something to think about.

[galyfray]

I think perhaps these new methods should be using doubles instead of floats.

done this is indeed better

[warjort]

they are used in the update method

I mean nothing overrides those methods. They always use the default method which just returns true.

[galyfray]

I mean nothing overrides those methods. They always use the default method which just returns true.

That's just me who plan to do proper things and forget the Large turbine should use it

[galyfray]

The ThrottableMultiblockController looks to me like it would be better implemented as a kind of throttle widget rather than a subclass? Similar to how things like filters fit into covers.

I know this is not how the LargeBoiler does it but it is something to think about.

currently only multi-block can throttle and I think this will stay. To my mind it's easier to change the class you extend and put a little of code in the workable handler than using a widget.

[galyfray]

I mean nothing overrides those methods. They always use the default method which just returns true.

The large turbine now use it as always intended

[warjort]

currently only multi-block can throttle and I think this will stay. To my mind it's easier to change the class you extend and put a little of code in the workable handler than using a widget.

Yes but it limits its reusablity, e.g.

• Adding throttling to things like the single block generators
• Adding throttling to a multiblock that needs to extend a different controller abstraction

But since this is implementation detail in gregtech.common somebody can probably change it later as usage requires.

[galyfray]

The other change (which I personally don't like) is the slicing of recipes into smaller pieces to make the throttle more responsive.
Besides not respecting the recipe configuration it means changes to the internal state of FuelRecipeLogic including the NBT that gets saved.

the slicing will get removed as it does nothing on the responsiveness of the throttle this is just some remnant of an older try.

I don't think canProduce energy should be by default this.recipeDuration >0 as it doesn't represent if the machine can produce energy it represent if the recipe as ended. in order to clean the API part of this I think I will need to be able to override what decide if the recipe has ended or not.

I will close this current mess and go on a fresh start to make things cleaner much easier to understand and as little invasive as possible.

I think this PR will get splitted in 4 :

• making the FuelRecipeLogic more flexible and allowing for cleaner ovverride while keeping his default behavior has is
• updating the large turbine code according to the last PR and take care of not breaking any addons while doing so
• adding a functional and versatile throttle widget (or plugin dunno which word is the most appropriated) and change the large boiler throttle to the usage of this widget.
• adding the widget to the large turbine

this way PR should be smaller with much precise and defined goal moreover they should be easier to verify if they are not breaking anything.

[warjort]

I don't think canProduce energy should be by default this.recipeDuration >0 as it doesn't represent if the machine can produce energy it represent if the recipe as ended. in order to clean the API part of this I think I will need to be able to override what decide if the recipe has ended or not.

But that isn't how the turbine works. The turbine doesn't care if there is a recipe running when it produces energy.
It produces energy if the rotor is spinning (recipe or not).
It does care about the recipe when it is adjusting the rotor speed.

[galyfray]

But that isn't how the turbine works. The turbine doesn't care if there is a recipe running when it produces energy.
It produces energy if the rotor is spinning (recipe or not).
It does care about the recipe when it is adjusting the rotor speed.

well the idea is the to separate the recipe running and the power production by default canProduce energy would be isRecipeRunning and isRecipeRunning would be remaningTime > 0 allowing for more customization on what decide if the recipe is over, which is used on many mechanics and I will probably need to override this behavior while still providing the ability to the turbine to produce energy while there isn't any recipe running.

on the current code state of this PR the turbine stop producing power when no recipes are running yet another mistake I've done.

[LAGIdiot]

That plan with separating this topic to multiple PRs seems reasonable to me. It will be easier to define scope, expectancy and some outline. Also it will be less work on reviews. I think we should make sure that we have clear view and understand what we want to achieve before writing code as it will save lots of time.

[galyfray]

Okay so here is what I propose for the first PR :

• getRecipeDuration // recipe duration with all factors applied
• getRecipeDurationLeft // might be a usefull getter
• getFuelConsumption // equivalant to calculateFuelAmount(this.previousRecipe)
• canConsumeFuel // will be used in the update method
• canProduceEnergy // to allow energy production when there is no recipe runing or without fuel consuption
• isRecipeRunning // return this.recipeDurationLeft > 0 but can be overrided to include different element.
• getFuelConsumptionMultiplier // applied once by default in startRecipe.
• getStaticEnergyEfficiency // applied once on recipe start
• getDynamicEnergyEfficiency // applied every tick before producing energy
• getLastRecipe // to get the last used recipe may be usefull

[warjort]

There's kind of an issue here. It is hard to tell what is accessors (exposed for display to the user)
and what is policy that has an affect on the logic.

I would suggest using getXXX() for accessors and calculateXXX() for policy.
e.g. calculateFuelConsumptionMultiplier() makes it more obvious that overriding the method affects the logic.
I know not all the existing methods do that, but don't change the names of the ones that already exist.

getRecipeDuration // recipe duration with all factors applied

I assume this is just a helper for calculateRecipeDuration(lastRecipe)
But you haven't included a calculateRecipeDurationMultiplier() to be used in that method?

getRecipeDurationLeft // might be a usefull getter

I wouldn't introduce unused code. If somebody has a use for it they can add it.
In general, you want to expose as few implementation details as possible to make future changes easier.

getFuelConsumption // equivalant to calculateFuelAmount(this.previousRecipe)

Ok

canConsumeFuel // will be used in the update method

I don't understand the use for this? In the existing PR it is never overrridden, it is always true.

canProduceEnergy // to allow energy production when there is no recipe runing or without fuel consuption

Ok. You should mention that the actual energy is calculated via the existing getRecipeOutputVoltage()

isRecipeRunning // return this.recipeDurationLeft > 0 but can be overrided to include different element.

This already exists, it is called isActive() and has important uses throughout the api

getFuelConsumptionMultiplier // applied once by default in startRecipe.

Ok. But I would suggest calling it calculateFuelConsumptionMultiplier see above.

getStaticEnergyEfficiency // applied once on recipe start
getDynamicEnergyEfficiency // applied every tick before producing energy

This is the most important part of this PR. You need to define more concretely what this means.
i.e. If I override these methods what will it do?
Also call them calculateXXXX()

getLastRecipe // to get the last used recipe may be usefull

This already exists as a protected field

[galyfray]

I would suggest using getXXX() for accessors and calculateXXX() for policy.

👍

But you haven't included a calculateRecipeDurationMultiplier() to be used in that method?

// recipe duration with all factors applied

if this method already exist or will be added (don't remember ) it will be included.

I don't understand the use for this? In the existing PR it is never overrridden, it is always true.

just in case if someone wants to use it while trying to make this class more flexible I we should do it once for all. I don't know if a futur machine or an addon machine may want to disable his consumtion under specific circumstance or just if he want to do some computation right before the fuel gets consume (even if it may not be the best place )

Ok. You should mention that the actual energy is calculated via the existing getRecipeOutputVoltage()

No, getRecipeOutputVoltage() is just a getter it's startRecipe which compute the recipe output voltage.

isRecipeRunning // return this.recipeDurationLeft > 0 but can be overrided to include different element.
This already exists, it is called isActive() and has important uses throughout the api

I'm willing to use this function as a replacement for this.recipeDurationLeft>0 in all the if statement that use it becose I will need to override this behavior. Is this a correct use for the isActive ? as far as I know when a recipe end the tick after the machine check for another recipe to start but doesn't change the state of isActive.

This already exists as a protected field

yeah that's why I propose a getter might be usefull but as you said no unused code so it won't get added.

calculateStaticEnergyEfficiency return a scaling factor which will be by default, applied to the output voltage. this operation will take place in the startRecipe method and therefore will get called once every time a recipe start. it define a constant efficiency that could not be changed without reseting the recipe or at all.

calculateDynamicEnergyEfficiency return a scaling factor which will be called just before outputing energy to the hatch. hte factor will be applied to the recipeOutputVoltage field which is the cached value returned by startRecipe. this factor will also be applied in the getRecipeOutputVoltage() in order to return the effective power output of the machine.

[warjort]

No, getRecipeOutputVoltage() is just a getter it's startRecipe which compute the recipe output voltage.

Not for the current turbine. You are describing how the default FuelRecipeLogic works.

LargeTurbineWorkableHandler's update() method calls getRecipeOutputVoltage() every tick and outputs that amount of energy.
A subclass of the turbine in an addon could have overridden that method.

To make this backward compatible, you would need to do something like:

public FuelRecipeLogic.update() {
+             long outputVoltage = calculateRecipeOutputVoltage();
                if (energyContainer.get().getEnergyCanBeInserted() >=
-                    recipeOutputVoltage || shouldVoidExcessiveEnergy()) {
-                    energyContainer.get().addEnergy(recipeOutputVoltage);
+                    outputVoltage || shouldVoidExcessiveEnergy()) {
+                   energyContainer.get().addEnergy(outputVoltage);
}

protected long FuelRecipeLogic.calculateRecipeOutputVoltage() {
    return this.recipeOutputVoltage;
}

@Override
protected long LargeTurbineWorkableHandler.calculateRecipeOutputVoltage() {
    return this.getRecipeOutputVoltage();
}

Although, if you are certain nothing besides the turbine overrides getRecipeOutputVoltage(), you could just use that method directly instead of introducing a new method.

[warjort]

calculateStaticEnergyEfficiency return a scaling factor which will be by default, applied to the output voltage. this operation will take place in the startRecipe method and therefore will get called once every time a recipe start. it define a constant efficiency that could not be changed without reseting the recipe or at all.

calculateDynamicEnergyEfficiency return a scaling factor which will be called just before outputing energy to the hatch. hte factor will be applied to the recipeOutputVoltage field which is the cached value returned by startRecipe. this factor will also be applied in the getRecipeOutputVoltage() in order to return the effective power output of the machine.

Does that mean you are applying 2 scaling factors? First scaling it in startRecipe() and then again in update()
Is it recommended to only override 1 of these methods depending on your use case?

[warjort]

just in case if someone wants to use it while trying to make this class more flexible

Don't introduce unused features. Besides making the code more complicated and potentially adding unnecessary constraints for future changes, you have no way of testing if it works as intended.
It's also another potential vector for a bug.

1st law of Programming: If its not tested it doesn't work. :-)

[warjort]

I'm willing to use this function as a replacement for this.recipeDurationLeft>0 in all the if statement that use it becose I will need to override this behavior. Is this a correct use for the isActive

It is the other way around. The active status is changed based on the recipeDurationLeft (as well as being under external control).
See IWorkable, IControllable, FuelRecipeLogic.setActive(), FuelRecipeLogic.update() and LargeTurbineHandler.update()

It is this active status that the rotor handler uses for its behaviour as well as things like lighting up the controller as feedback to the user.

You don't explain why you need to change it?

[galyfray]

To make this backward compatible, you would need to do something like:

Or I can just say : getRecipeOutputVoltage() is meant to return the output voltage of the recipe so using it directly will be completely harmless except for the people who have changed it to not return the output voltage of the recipe which I hope no one did because it would be very weird .

Does that mean you are applying 2 scaling factors? First scaling it in startRecipe() and then again in update()
Is it recommended to only override 1 of these methods depending on your use case?

Yes I apply both of them. the static computation is here for efficiency if you have any computation and your efficiency evolve at most once every recipe change it is recommended to override only the static. If you have an efficiency that need to be updated every tick then the dynamic version should be overrided now if your efficiency is compound of both par then override both methods. the main idea is to try to keep things as light as possible by removing useless /unneeded computation.

Don't introduce unused features. Besides making the code more complicated and potentially adding unnecessary constraints for future changes, you have no way of testing if it works as intended.
It's also another potential vector for a bug.

as said we should do it once so no one would need to do the job another time we are talking about something in the update method of class that's not an easy task to override this part of the behavior in a clean way.
Moreover I would like to split the fuel consumption from the energy production and from what append if a recipe is or not running thus the creation of this function won't be meaningless. In addition canProduceEnergy exist it would be weird that canConsumeFuel doesn't.

1st law of Programming: If its not tested it doesn't work. :-)

come on I don't think it is not possible to not test it at first and I'm not sure that testing an if statement with a true inside make sense x) still we could make a button in a turbine to switch it and see how that it runs without any fuels to test it if you absolutely want to test it ;-)

It is the other way around. The active status is changed based on the recipeDurationLeft (as well as being under external control).
See IWorkable, IControllable, FuelRecipeLogic.setActive(), FuelRecipeLogic.update() and LargeTurbineHandler.update()

this is why I proposed another function that work the way I want even if as you said something which heavly looks like what I want exist we should find a way to make a clear an understandable difference between those two methods.

You don't explain why you need to change it?

yes it was obvious in my mind but as you're not in my mind it is not. In the current try I changed recipeDurationLeft by a float and changed all this.recipeDurationLeft>0 by this.recipeDurationLeft> getConsumptionMultiplier() so as as far as I know I will have to do it again bu only on the large turbine side I would like to be able to do it in a much cleaner way.

[galyfray]

current debate state :

new methods which looks ok so far

• calculateRecipeDuration // equivalent to calculateRecipeDuration(lastRecipe)
• calculateFuelConsumption // equivalant to calculateFuelAmount(this.previousRecipe)
• canProduceEnergy // to allow energy production when there is no recipe runing or without fuel consuption
• calculateFuelConsumptionMultiplier // applied once by default in startRecipe.
• calculateStaticEnergyEfficiency // applied once on recipe start
• calculateDynamicEnergyEfficiency // applied every tick before producing energy
• calculateRecipeOutputVoltage // by default behave just like getRecipeOutputVoltage() but will include calculateDynamicEnergyEfficiency and return effective power output while getRecipeOutputVoltage() will be stay a getter (see below for more explanation)

new methods I forget to put in the list and didn't understand that warjot was pointing this out (guessed as ok) :

• calculateRecipeDurationMultiplier() // applied in calculateRecipeDuration(lastRecipe)

new methods still in debate

• isRecipeRunning // return this.recipeDurationLeft > 0 but can be overrided to include different element. name is not final
• canConsumeFuel // will be used in the update method

more detail on some methods :

isRecipeRunning():
In the current try I changed recipeDurationLeft by a float and changed all this.recipeDurationLeft>0 by this.recipeDurationLeft> getConsumptionMultiplier() so as as far as I know I will have to do it again bu only on the large turbine side I would like to be able to do it in a much cleaner way. this is the main aim of this method which is not equivalent to isActive() even if there are really close. Any better name enhanced behavior idea or different way of doing this would be appreciate.

calculateStaticEnergyEfficiency and calculateDynamicEnergyEfficiency
They both represent efficiency and only calculateDynamicEnergyEfficiency is needed to correctly achieve to apply an energetic efficiency. calculateStaticEnergyEfficiency is called once per recipe and is therefore lighter than calculateDynamicEnergyEfficiency which will be called each time energy can be produce (once per tick by default). calculateStaticEnergyEfficiency will be a part of startRecipe whereas calculateDynamicEnergyEfficiency will be a part of calculateRecipeOutputVoltage(). To make it short if it can change while the recipe is running => calculateDynamicEnergyEfficiency else calculateStaticEnergyEfficiency.

planned changes to the updates method (not yet approved) :

the update part will be splitted in three big part :

• Energy
• Fuel
• Recipe

Energy
in the Energy part energy will be produced if and only if canProduceEnergy is true the amount produced could not be given by getRecipeOutputVoltage() because as warjot pointed out this method is already override in the large turbine in a way such that if we use this method in our new update behavior the turbine will produce power twice. As proposed by warjot using calculateRecipeOutputVoltage() to compute the power output and adding a docstring to tell that getRecipeOutputVoltage should be a getter not overrided and calculateRecipeOutputVoltage() should be overrided instead and used to get the effective power output. If the planned changes aren't applied in let's say an add-on things should not break as by default the two methods will behave in the same way.

Fuel
in the Fuel part this.recipeDurationLeft will be decreased by one if canConsumeFuel is true

Recipe
the Recipe part is more about checking if a recipe is running trying to get a new a new recipe if possible and calling setActive when needed. trying to find a new recipe will be done if the current recipe has ended and canConsumeFuel is true as if we cannot consume fuel there is no need to setup a new recipe we should continue with the current one has if it hasn't ended yet. not taking in account canConsumeFuel in this part is just a matter of way of thinking the recipe part.

this is I think the current state of the planning of the first PR. Tell me if you think I have misunderstand something (I do it too many time ) or missed something.

[warjort]

I was looking at the original feature request and what that says makes more sense to me than what you are trying to do?

Add buttun for adapt your large turbine for your power consumption or fuel (steam, gas or plasma) production.

Add button for increase or decrease large turbine max speed

Though I don't think that request is entirely correct.

What this should be doing for the turbine is to have your single widget for the throttle%
But the throttle% for the turbine should be applied to the recipe duration, consumption and the max speed of the rotor, not the output voltage directly.

If say you have 50% throttle, you consume less fuel, but the recipe doesn't last as long, so you get less ticks on the rotor to accelerate it.
But it also means the maximum speed of the rotor is adjusted down as well, meaning the maximum energy is reduced.

I don't think for the turbine you should be doing anything to the output voltage directly. It should just be left as it currently is based on the rotor speed and quality.

Having a multiplier for the output voltage still makes sense for more traditional generators like the diesel generator, where the output voltage is directly related to the fuel.

[galyfray]

according to what I remember the throttle doesn't change the boiler max temp the throttle is applied to the fuel and the steam output which in the case of the turbine I transpose the steam output to the power output which makes perfectly sense.

after checking the throttle change the recipe duration which makes no sense to my mind. let me explain : when we throttle we expect to produce less but we also expect to consume less with a loss of efficiency which is displayed on the display but with the current implementation the throttle only change the output keep consuming as much as if running full throttle and add a efficiency loss over this.

I fact I'm asking myself if the current throttle does change something to the output

[warjort]

when we throttle we expect to produce less but we also expect to consume less with a loss of efficiency

In principle, reducing the throttle should mean you can do more work (total energy), i.e. it is more efficient.
The down side is it takes longer because the power (energy per second) is reduced.

Running at full throttle lets you do things quickly but at reduced efficiency and therefore more fuel consumption.

Think about driving a car in first gear. You could do the whole journey in first gear which would use less fuel, but it would take forever because you can't accelerate to any great speed.

In practice, things are usually more complicated than this simple (spherical horse in a vacuum) explanation. :-)

[warjort]

I transpose the steam output to the power output which makes perfectly sense.

But I am arguing it doesn't make sense. The correct transposition is steam output -> rotor accelarations.
Altering the throttle of the boiler doesn't change the energy produced by the steam (it changes the steam production).
Nor should the throttle on the turbine change the energy produced by the rotor speed (it changes the rotor accelaration).

[galyfray]

with the current implementation everything gets decrease by the throttle multiplier the total output per recipe duration is lowered (if not the current implementation does not work) but the fuel used per recipe duration isn't lowered as both get multiplied by the throttle multiplier leading to the conclusion that the total output per fuel used is lowered meaning the current implementation heavily decrease the efficiency of the boiler.

the rotor acceleration is more like the boiler heat has you consume less fluid you produce less power think in term of real life when we want to throttle our turbine we do not reduce there speed we reduce how much steam we send to the turbine and lower how much power is consumed (it's the other way around the power consume first decrease then the steam send but that's the same behavior in the end) by reducing how much steam we produce per tick we reduce how much power we generate per tick not how heat is the boiler.

[warjort]

I can't say I am big expert on the algorithm in the LargeBoiler. I have never used this feature.
So the actual tuning of its multipliers I can't comment on and they are not really relevant to my argument.

What is relevant is that a multiplier exists which affects the duration of the recipe.
While the controller/recipe isActive(), there is a direct correspondance between the large boiler and the turbine.
The steam boiler does ++currentTemperature every 20 ticks, while the turbine's rotor handler does incrementSpeed() every tick.

So changing the duration, changes the number of isActive() checks that returns true, which then changes the number of temperature or rotor speed increments.

How the consumption multiplier and other multipliers change with respect to the duration multiplier is just a tuning issue.

[warjort]

The correct transposition is steam output -> rotor accelarations.

So my previous comment was misleading. It is temperature -> rotor speed that is the correct transposition.

[galyfray]

What is relevant is that a multiplier exists which affects the duration of the recipe.
While the controller/recipe isActive(), there is a direct correspondance between the large boiler and the turbine.
The steam boiler does ++currentTemperature every 20 ticks, while the turbine's rotor handler does incrementSpeed() every tick.

the current implementation in the large turbine doesn't change the duration of the recipe I don't really see a way of changing the recipe duration while keeping the power/fuel ratio stable before applying throttle efficiency.

still you make a point and this may be intresting to take this in account

[warjort]

the current implementation in the large turbine doesn't change the duration of the recipe

That is the feature you are adding. :-)

I don't really see a way of changing the recipe duration while keeping the power/fuel ratio stable before applying throttle efficiency.

This should all come out in the wash when you change the consumption based on the throttle.
The simplest implementation is to keep the efficiency the same. i.e. apply the same multiplier to the duration and consumption
but we can discuss what the real function should be seperately.

[warjort]

The other issue is the third component of the throttle.

For the large boiler this applies a steam output multiplier.

So the question is should this do something similar in the turbine, i.e. apply voltage output multiplier as you originally implemented it, or should it be as the feature request suggests, a multiplier applied to the max rotor speed.

From a physical point of view, what the throttle is really doing is changing the flow of fuel through the rotor.
So it would make sense that a reduced flow means you can't reach higher speeds.

[galyfray]

the current implementation in the large turbine doesn't change the duration of the recipe

That is the feature you are adding. :-)

well humm yes but actually no I'm dinamicaly changing the how fast is consumed the duration to balance throttle change by artifficially increasing the effecive consumption

The simplest implementation is to keep the efficiency the same. i.e. apply the same multiplier to the duration and consumption
but we can discuss what the real function should be seperately.

I define efficiency as power/fuel so it would be consumption and and production but yeah this something we should discuss when we would be planning the 4th PR

[galyfray]

So it would make sense that a reduced flow means you can't reach higher speeds.

yes but not mendatory if you produce less power you produce less resistance to the turbine moovement so you can still run as fast as previously. Once again think about turbine in real wolrd we keep them at a stable speed no matter how much they produce. We do this because the stability of the AC frequency is important but I here mean that it is possible and still logical to keep the same speed while reducing both power output and fluid input.

[LAGIdiot]

I will start stating that I am not expert on turbines or driving (either car or spherical horse in a vacuum). And I think that most of our players are not too (at least first part) so mechanic we come up has to be ease to understand for everyone. Without need to explain it through many paragraphs of text full of mathematical formulas. If we can't we are not touching this!

How I think turbines should work:
If turbine is spinning fast enough it produces energy. Faster it goes more energy is produced.
If there is enough fuel turbine is gaining speed. If there is not, speed is going slowly down.
Speeding/slowing is not linear. It takes more effort (energy) to speed up turbine form 0 to 1000 then from 1000 to 2000. Slowing is vice versa.
Not all fuel is able to spin turbine to maximum (as it does not have enough punch).
Rotor efficiency is used to calculate produced energy and increase in speed of turbine.
Throttle I see as how much % of fuel we insert. Which will affect speed gain and maximum speed of turbine. Theoretically we could even go higher then 100% (with some bad effects).

Now this is probably not accurate to real world. But it does not have to be 100% (of course it should not be total bullshit) this is not simulator it is game mechanic to make it fun. Also just because I said how I think it should be does not mean anyone will go implement it that way. This is open for discussion and I believe that other GTCE Council members will have something to say.

Also for all discussion compatibility should be in mind.