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

Added block data to FakeBlockAction and ShowGen, so BlockStates are saved and loaded. #1

Open
wants to merge 9 commits into
base: staging
Choose a base branch
from

Conversation

KyGuy2002
Copy link

The FakeBlockAction currently doesn't allow BlockStates such as orientation, faces, open/closed, etc. This causes these blocks to have the default states, which looks bad in many situations.

I have added block states to showgen, and parse them as with the other values.

Currently Saved States:
Stairs
Doors
TrapDoors
GlassPane
Fence

Other Small Changed:
In addition, I fixed the showgen by flipping a >= to <=, not sure why it was like this in the repo...

Also, the auto-update message on join wasn't working due to its listener not being registered, so that's fixed.

I updated the dependencies and incremented the minor version number.

@CubitsDev
Copy link
Owner

@KyGuy2002 Is this good for reviewing? Just seen you added another commit so want to check before continuing.

@KyGuy2002
Copy link
Author

Yes this should be all ready, I have tested it “in the lab” on my server but am yet to use it in a full scale show. I will be setting up a larger show soon, so maybe wait until then just incase. I will update you when I have done that, but if you want I can always just do another PR.

Also, I’m going to be making another PR for improved docs soon as well.

@KyGuy2002
Copy link
Author

Alright, I tested it again, seems like it works as intended. I will probably update it again later with some more supported blocks and a few other things I noticed, but idk when I will do that. Its ready for review.

@KyGuy2002
Copy link
Author

Any update @CubitsDev ? This, #2 and #4 are all ready for review. Thanks

@CubitsDev CubitsDev changed the base branch from master to staging April 22, 2022 09:54
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
dataString = new StringBuilder("FENCE,");

// True for included
for (BlockFace face : ((Fence) blockData).getFaces()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be switched to a switch case statement to avoid the if else if repetition.

Copy link
Author

@KyGuy2002 KyGuy2002 Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your meaning the whole part, or just the // True for included part?

Im using instanceof so i dont think its possible to do that in a switch statemnt

src/main/java/network/palace/show/utils/ShowUtil.java Outdated Show resolved Hide resolved
src/main/java/network/palace/show/utils/BlockDataType.java Outdated Show resolved Hide resolved
@KyGuy2002
Copy link
Author

I resolved all requested changes, and left a response to one of them @CubitsDev . Thanks

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