WIP: Modem peripherals toggling #510
Conversation
|
Looks like #264 is related. |
|
Regarding API. |
SquidDev
added
area-Minecraft
There was a problem hiding this comment.
ModemPeripheral/ModemState does weird things with TickScheduler because we don't actually care about what's done on the main thread - it's only there to update the visual state.
In our case, we do probably care about the result of togglePeripheralAccess - it'd be nice if we returned whether peripherals were enabled or not (or something). In this case, we don't need to do anything with AtomicBooleans or anything - just add a @LuaFunction( mainThread = true ) on togglePeripherals and you should be safe to call the TE method directly.
As far as naming goes, I'm not too sure. Definitely agree that either is/set or is/enable/disable is the way to go. Not really sure what the best name is though - "Active" would be one candidate I guess, but it's not super clear.
| @@ -61,4 +61,9 @@ public void networkChanged( @Nonnull IWiredNetworkChange change ) | |||
| protected abstract void attachPeripheral( String name, IPeripheral peripheral ); | |||
|
|
|||
| protected abstract void detachPeripheral( String name ); | |||
|
|
|||
| public void togglePeripheralAccess() | |||
There was a problem hiding this comment.
Probably worth making this abstract instead, though I realise this may just be a placeholder while we work through the design :).
|
Interesting thing happens. With full block modems everything works as expected, but with cable modems it's only possible to share a computer directly attached to the modem. Turtles don't see cable modems as peripherals at all. Therefore they can't call any methods. Probably some |
Wired modems shouldn't be able to attach to turtles at all, as turtles aren't a full block, so I don't really think this is an issue. |
|
I agree, turtles shouldn't use cable modems as modems. But they should be able to activate/deactivate neighbor modem as part of construction process. This could be a new type of peripheral, or may be possibility to |
| { | ||
| if( sharingPeripherals == enabled ) return sharingPeripherals; | ||
| sharingPeripherals = enabled; | ||
| if( updateTile ) |
There was a problem hiding this comment.
Doing this as a boolean flag feels super janky. It's probably worth having two methods, where one of them is a trySetSharingPeripherals or something.
There was a problem hiding this comment.
Done. Now it's two methods.
|
Alternatively, |
|
if this is WIP, shouldn't this be a draft PR? |
|
Thinking of picking this up, am I right in thinking this issue in the mod was never resolved? |
|
correct. no such feature exists in the main mod. |
|
In #1469,
I suppose this PR can be closed too. |
|
This PR adds peripheral methods to the modem themselves, not the turtle. This is mostly stalled as I'm still not 100% convinced on the idea, but not against it either. |
|
Maybe turtles could toggle modems by using |
I would like to toggle modems by turtles to automate building.
Regarding the code: it works. Only with fullsized wired modems.
There's nothing else which could be called good, since I'm complete newbie in modding, in CC codebase and in Java to complete the picture. Therefore I'm ready to be kicked for bad code/patterns/structure.
I'm planning to add similar thing to usual wired modems and to think more about Lua API. And of course I'm open for discussion.