search

AlmuraDev / SGCraft

Stargates mod for Minecraft
https://www.almuramc.com/sgcraft/SGCraft.html
MIT License
31 stars 31 forks source link

Incorrect CC API implementation for getPeripheral() #151

Closed LemADEC closed 4 years ago

LemADEC commented 4 years ago

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.

Dockter commented 4 years ago

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.

LemADEC commented 4 years ago

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

Dockter commented 4 years ago

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

Dockter commented 4 years ago

Give this a try: http://solder.almuramc.com/downloads/SGCraft-2.0.4-beta-4.jar

Dockter commented 4 years ago

Fixed.

LemADEC commented 4 years ago

Looking good, remember to comment the printf in https://github.com/AlmuraDev/SGCraft/blob/dea61cdc9fcf87981e3cc3df3f13c55a97b926ad/src/mod/gcewing/sg/features/cc/CCSGPeripheral.java#L143-L153

Dockter commented 4 years ago

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