Flawed AE2 integration
yueh opened this issue ยท 2 comments
Due to a random issue (proved to be unrelated), I glanced over your implementation for the AE2 integration and noticed af few issues with it.
First of all TileMETransformer.java#L44-L52 is flawed as calling IGridNode.updateState()
on your neighbors is potentially a bad idea.
It violates the contract which states, it should be called when your state changes, but does not mention you should or must call it on any of your neighbors.
Doing so has a couple of issues, like it might not even be called on an IGridNode
connected to your tile entity, causing a completely unrelated network to repath. Also it might cause a neighbor IGridHost
to join a grid too early and in an incomplete state, resulting in random issues. Currently I do not know of any IGridHost
implementation delaying their join, but that does not mean some addon might do it at one point. AE2 is perfectly fine should a IGridHost
decides to only join a grid 5 minutes after it was placed into the world.
Worst case is probably that an IGridNode
with an undetermined state joins prematurely, crashes and then saves an unrecoverable state to disk without any chance to recover later. Probably very unlikely, but I would not be surprised, if one addon would do it at some point.
Secondly TileMETransformer.java#L54-L62 is a bit complex with manually creating a grid connection between the two parts. Changing TileMETransformer.java#L126-L128 to return an EnumSet
containing UP
for the lower and DOWN
for the upper part and let AE2 handle the connection is probably easier.
Thirdly TileMEWireConnector.java#L49-L60 should not use an internal class.
IAppEngApi.createGridConnection()
will already take care of checking, if a connection already exists and throw an exception accordingly. Also just log the message and not the full stacktrace, so it is consistent with our logging. With rv3 calling GridNode.hasConnection()
is even no longer possible.
Last but not least using WorldSettings
in BlockMEWireConnector.java#L27, BlockMETransformer.java#L67 and BlockMETransformer.java#L74 is not ideal. Just use the registry provided by IAppEngApi
, which would also allow you to support rv2 and rv3 without any change (and probably earlier ones). Otherwise it can break with any build of AE2, even stable ones as we do not guarantee any internal implementation to be stable.
Edit:
Regarding 2. You could even consider reducing it to a single IGridNode
in the bottom part and "connect" the wire directly to this one. Saving one IGridNode
and having to deal with a multiblock with multiple world accessible ones.
Thanks for the advice, had no experience with AE prior to this so tbh I am happy it even worked, I will address these soon
If in doubt, just ask us. Probably easier then trying to figure it out.
AE2 has a pretty different design compared to other APIs like RF and we had a couple cases where devs still wanted to apply the concepts they know from these.
The basic concept is that a tile entity implementing IGridHost
announces that it wants to be part of a grid and provides an IGridBlock
to define its properties like on which sides it allows connections.
IGridBlock
even does not need to be something like a Block
or TileEntity
at all, a simple class implementing this interface is sufficient. The IGridNode
is then the entry point for anything a grid provides. Everything else like actually forming connections is handled by AE2 itself (basically forming something like a large interdimensional multiblock).
Manually creating connections is only needed to bridge gapes, like the wires for IE.
Due to this AE2 simply has nothing like canConnect
, because this is basically fullfilled once one or more IGridHost
exists in any dimension including itself. Thus always true, as it is a valid use case that a single IGridHost
could create a connection between two or more sides to say tunnel an external grid/network without letting it join the same one as the tunnel itself.