AlmuraDev/SGCraft

Incorrect CC API implementation for getPeripheral()

Closed this issue · 7 comments

Since at least 2.0.2, the mod reports a different object for each side of a peripheral while the CC API expects the same object returned for all faces of that object.
See initial logs from #88 about this: when placing a second computer, there's extra attach & detach events popping up. Here's logs from my mod, then from that build of SGCraft:

[07:57:32] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: test WD
[07:57:33] [Server thread                  /INFO] [warpdrive]: [CC] IPeripheralProvider.getPeripheral @ Spacheese (265 71 234) west TileEntityAirGeneratorTiered '' @ Spacheese (265 71 234)
[07:57:38] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: Computer 1 placed
[07:57:39] [ComputerCraft-Computer-Runner-0/INFO] [warpdrive]: [CC] Attaching warpdriveAirGenerator @ Spacheese (265 71 234) with 429:right
[07:57:44] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 1 opened
[07:57:45] [Server thread                  /INFO] [warpdrive]: [CC] IPeripheralProvider.getPeripheral @ Spacheese (265 71 234) east TileEntityAirGeneratorTiered '' @ Spacheese (265 71 234)
[07:57:49] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: Computer 2 placed
[07:57:50] [ComputerCraft-Computer-Runner-0/INFO] [warpdrive]: [CC] Attaching warpdriveAirGenerator @ Spacheese (265 71 234) with 430:left
[07:57:50] [Server thread                  /INFO] [warpdrive]: [CC] IPeripheralProvider.getPeripheral @ Spacheese (265 71 234) west TileEntityAirGeneratorTiered '' @ Spacheese (265 71 234)
[07:57:54] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 2 opened
[07:57:56] [ComputerCraft-Computer-Runner-0/INFO] [warpdrive]: [CC] Detaching warpdriveAirGenerator @ Spacheese (265 71 234) from 429:right
[07:57:56] [Server thread                  /INFO] [warpdrive]: [CC] IPeripheralProvider.getPeripheral @ Spacheese (265 71 234) east TileEntityAirGeneratorTiered '' @ Spacheese (265 71 234)
[07:58:00] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: Computer 1 broken
[07:58:02] [ComputerCraft-Computer-Runner-0/INFO] [warpdrive]: [CC] Detaching warpdriveAirGenerator @ Spacheese (265 71 234) from 430:left
[07:58:05] [Server thread                  /INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: Computer 2 broken


[08:00:29] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 1 placed
CCSGPeripheral.attach: to dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@277eba66
[08:00:34] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 1 opened
[08:00:39] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 2 placed
CCSGPeripheral.attach: to dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@ddba733
CCSGPeripheral.detach: from dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@277eba66
CCSGPeripheral.attach: to dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@1399973e
[08:00:45] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 2 opened
CCSGPeripheral.detach: from dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@1399973e
CCSGPeripheral.detach: from dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@ddba733
CCSGPeripheral.attach: to dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@3d7dda88
[08:00:51] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 1 broken
CCSGPeripheral.detach: from dan200.computercraft.core.apis.PeripheralAPI$PeripheralWrapper@3d7dda88
[08:00:56] [Server thread/INFO] [minecraft/DedicatedServer]: [SAW] Admin LemADEC: computer 2 broken

SGCraft implementation of getPeripheral should cache previously created interfaces, or, just use the TileEntity itself as an interface or storage of the latest.
For reference, getPeripheral() is called from the main thread so the World.getTileEntity() call is safe.

According to this: https://github.com/AlmuraDev/SGCraft/blob/master/src/mod/gcewing/sg/features/cc/CCPeripheralProvider.java#L19

The lookup returns the same TE regardless of face so I'm not sure exactly what you're saying the problem is.

According to the exact next line, a new object is created for each call. CC API doesn't expect that.

Oh, I see what you're saying..... hm.

Fixed.

Looking good, remember to comment the printf in

System.out.printf("CCSGPeripheral.attach: to %s\n", cpu);
this.world.getMinecraftServer().addScheduledTask(() -> {
CCInterfaceTE te = getInterfaceTE();
if (te != null) {
te.attachedComputers.add(cpu);
}
});
}
public void detach(IComputerAccess cpu) {
System.out.printf("CCSGPeripheral.detach: from %s\n", cpu);

That got missed in the official 2.0.4 build, I'll go comment it out so its ready for the next.