Support for IO modes in MjAPI

[Prototik]

No description provided.

[SpaceToad]

I think this one is going one notch too far. I'm assuming that you're developing your own energy specialization and that you would like to have additional properties in the annotation, right? In the current state of things, only the interface-based model will give that level of flexibility,

[Prototik]

I'm not developing my own energy system, but I want to provide necessary things which available in other power systems in BC's MjAPI. I think that basic input/output configuring should be available in the bc api.

[SandGrainOne]

I don't know exactly what SpaceToad is planning for the Power API, but it was never meant to be a framework for new power systems. BuildCraft power is a distinct power type and the API is there so other mods can use and produce BuildCraft power.

[Prototik]

I did not create a new power system. I want to use BC's MJ, but with using some expected behavior, e.g. bottom side of block receive power, or up side emit power.
IO configuring is exists almost in every other energy framework for minecraft (look for Thermal Expansion, Ender IO, Gregtech), but not in BC.

[SpaceToad]

@SandGrainOne it actually initially was - there was even two power systems provided in the 3.x version of BuildCraft.

@Prototik ok - I understand where you're coming from. At this stage though, MjBattery is only used for tiles receiving energy. Do you have an example of the "send" mode to share?

[Prototik]

@SpaceToad for example engines (which has an internal buffer for energy), capactior banks, converters /etc...

[SpaceToad]

@Prototik ok - could you show what would be the new implementation of the engine if using this mechanism?

[Prototik]

@SpaceToad implementation should not changes (at least now), iomode only for meta-information to other parts of energy framework (eg power extraction pipes could extract energy from battery which marked for send support)

[SpaceToad]

@Prototik is the idea behind that to implement a pull power model as well as a push one? That is, for now, power is pushed to machines, but with this features, machines could look around and pull power for themselves?

[Prototik]

@SpaceToad Nope, real behavior should be controlled by transport system (in BC - pipes), iomodes for support handling this.

[SpaceToad]

So pipes would pull power from tiles with "out" mode and push to tiles with "in" mode?

[Prototik]

@SpaceToad yes, almost ideal solution.

[SpaceToad]

@Prototik I like this. It simplifies a lot third-party implementation of power production system. Are you working on an update of the wooden kinesis pipe for that?

[Prototik]

@SpaceToad yep, but following the principle "one PR - one feature" this PR for IO modes :)

[SpaceToad]

@Prototik that's good policy.

Carrying on the review, I noticed that you introduced a lot of local variables

    private double maxCapacity;
    private double maxReceivedPerCycle;
    private double minimumConsumption;
    private ForgeDirection[] sides;
    private String kind;
    private boolean batteryDataOverrided = false;
    private IOMode mode;

But they're not read most of the time

public double getEnergyRequested() {
        if (!mode.canReceive) {
            return 0;
        }
        try {
            return JavaTools.bounds(batteryData.maxCapacity() - energyStored.getDouble(obj),
                    batteryData.minimumConsumption(), batteryData.maxReceivedPerCycle());
        } catch (IllegalAccessException e) {
            BCLog.logger.log(Level.WARNING, "can't get energy requested", e);
        }
        return 0;
    }

They seem to be only used for the PowerAPI compatibility. How about keeping the main BatteryObject with the minimal fields and the annotation, and sub-classing just for usage in PowerAPI?

[Prototik]

@SpaceToad it's using for overriding an MjBattery annotation.
I've trying to minimize this implemntation...

But they're not read most of the time

They reading in overrideBattery()

[SpaceToad]

@Prototik oh, I seen. In this case, it's a bit of a bummer to store extra data if they don't have anything to do with the state of the object. They should really be passed as parameters of overrideBattery.

[Prototik]

Is that better?

[SpaceToad]

@Prototik definitely. One last comment, but I'll take care of this upon merge. Receive mode feels like a more natural default than Both. Default scenario is somebody using the annotation to receive power, not changing anything, and allowing the default to operate as a battery is a bit too much.

[SpaceToad]

Also, I'd really like to try an API freeze sometime today, with a 6.0.12. Except if there's any surprise left with that part of the builder framework. Let me know if you see any substantial hole left in the Energy API.

[Prototik]

PR updated.

I have only one problem - if i connect multiple engines to one block from different sides - total mj/t ratio can be more then maxReceivedPerCycle (up to 6). But it's transport (or BatteryObject implementation) issue, not the API. I even have a solution for this.

[SpaceToad]

BatteryObject implementation is part of the API, so changing it will break the freeze. The attempt at freezing is that we don't change a line of code in the API packages.

I see the problem you're pointing out though. We don't control max received per cycle but only per input side.

Anyway, merging in your change, and thinking further on that one.