Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Returning a machine status in tickDisabled #9

Conversation

supersimple33
Copy link
Contributor

Presently, if you switch a machine to the high requirement for Redstone the machine will not change its status based on this selection. I have added the return type of MachineStatus to the tickDisabled method. I have set the default behavior to return the status of MachineStatuses.OFF. I decided to do this rather than leave tickDisabled alone since those that overide tickDisabled may have disabled their machine for reasons other than Redstone and wish to set a different status for the machine.

@supersimple33
Copy link
Contributor Author

supersimple33 commented Jul 17, 2023

WRT 1296060: I need access to the redstone configuration on the client side for some fixes I am doing to the bubble distributor. To do this I am calling sendBlockUpdated when setRedstone is called so that the client machineBlockEntitys are properly updated. Could you please double-check that I wrote this impl correctly and that Block.UPDATE_IMMEDIATE is the right arg for this.

@marcus8448
Copy link
Member

The redstone sync changes look good but I'm not exactly sure how the OFF status helps: what other reasons should machines be disabled beyond redstone control?

Disablement is effectively 'pausing' the machine - when the machine gets re-enabled, it should act like nothing happened. tickDisabled exists to allow machines that have a in-world side-effect (e.g. oxygen sealer) to suspend their effects.

Setting the OFF status might affect machines that rely on the status being set for logic/state control (machines don't know when they're going to be disabled).

@supersimple33
Copy link
Contributor Author

The redstone sync changes look good but I'm not exactly sure how the OFF status helps: what other reasons should machines be disabled beyond redstone control?

Yeah I can't think of any reason off the top of my head but I left it ambiguous so that when add-ons come along it's clear where they would integrate. I mean this variable could just as easily be called DISABLED.

Disablement is effectively 'pausing' the machine - when the machine gets re-enabled, it should act like nothing happened. tickDisabled exists to allow machines that have a in-world side-effect (e.g. oxygen sealer) to suspend their effects.

Yeah so these changes are necessary for making the O2 sealer work (TeamGalacticraft/Galacticraft#277). In GC4 we had a button inside the o2 sealer where you could turn it off so that you would not kill your o2 storage and in GC5 this would be done by placing a lever next to the sealer so that you can flip the lever as needed to get oxygen. The reason I need these changes is that if you disable a machine then it does not update the UI since it is left with the same status. If you add the following code to the Generator Test Machine you will see that the status is not properly updated and thus may leave players confused as to why they are suffocating when the status is sealed. Further since the sealer doesn't have a visual effect on the world (unlike bubble distributor) it makes sense to have a dialog inside the machine which spits out the status like the code below but without these changes the machine will say it is safe to breath when it is not.

@Override protected void renderForeground(GuiGraphics graphics, int mouseX, int mouseY, float delta) { super.renderForeground(graphics, mouseX, mouseY, delta); graphics.drawString(this.font, Component.translatable("ui.machinelib.machine.status").append(this.menu.state.getStatus().getText()), this.leftPos + 50, this.topPos + 30, 5592405, false); }

Setting the OFF status might affect machines that rely on the status being set for logic/state control (machines don't know when they're going to be disabled).

What do you mean here as why would one need to know the Status of a machine which has been disabled as there are no guarantees the player hasn't moved items around in the machine which would change the status when the Machine is flipped back on.

@supersimple33
Copy link
Contributor Author

I figured out how to do what I needed without these changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants