-
Notifications
You must be signed in to change notification settings - Fork 205
Spray can refactor and creative spray can #2858
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
base: master
Are you sure you want to change the base?
Conversation
…c) but usable items (spray cans, lighters, etc) weren't accounted for
…tching to sooth my soul
…a mouse button or scroll event happened (looking around emits an event per tick, too many packets!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Color name in the tooltips of the spray cans should be colored the appropriate color
The default Creeative Spray can in JEI has an empty NBT tag on it, which should probably be removed. It also defaults to Solvent, whereas I would rather have it default to one of the existing colors, or have some tooltip saying that is can provide all colors.
When testing on Hardened Clay (Terracotta), if I use solvent on a block, I can then not recolor the block using a spray can. Seems like Vanilla Blocks only
In the toolbelt, the Durability bar of the default creative spray can from JEI goes over 1 slot width.
Using a spray can from the toolbelt transformed the spray can into a Copper Credit after one use.
The pipe walker from the linked superseded PR has some issues (It did in that PR as well). When arranging pipes into a +, not all branches of the net are walked and colored. When the pipes are set up like image, the pipe walk is not performed, and the pink pipe is not colored

When a spray can runs out of contents, an empty tag is left on the canister. This tag should probably be removed somehow when the last spray is used.
| try { | ||
| // try to read the default color value from the default state instead of just | ||
| // blindly setting it to default state, and potentially resetting other values | ||
| defaultColor = (EnumDyeColor) defaultState.getValue(prop); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to null check this here at all, in case it is a mod doing something weird with properties?
| // blindly setting it to default state, and potentially resetting other values | ||
| defaultColor = (EnumDyeColor) defaultState.getValue(prop); | ||
| } catch (IllegalArgumentException ignored) { | ||
| // no default color, we may have to fallback to WHITE here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a line of info text into the log in this case, to catch common other mod blocks? Probably gated behind the debug config option.
| return GTGuiTheme.STANDARD; | ||
| } | ||
|
|
||
| // TODO: change to abstract once MUI2 port is complete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other Items that need a MUI2 GUI that can be opened from the hand?
| } | ||
|
|
||
| public static MapColor getMapColor(int rgb) { | ||
| public static @NotNull MapColor getMapColor(int rgb) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this go in ColorUtil now? As well as the other color stuff used for prospector stuff with the various map mods.
src/main/java/gregtech/common/items/behaviors/spray/DurabilitySprayBehavior.java
Outdated
Show resolved
Hide resolved
src/main/java/gregtech/common/items/behaviors/spray/CreativeSprayBehavior.java
Outdated
Show resolved
Hide resolved
src/main/java/gregtech/common/items/behaviors/spray/CreativeSprayBehavior.java
Outdated
Show resolved
Hide resolved
| if (pipeTile.getNumConnections() == 2) { | ||
| int connections = pipeTile.getConnections(); | ||
| connections &= ~(1 << facing.getOpposite().getIndex()); | ||
| for (EnumFacing other : EnumFacing.VALUES) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't the superseded PR create a custom pipe net walker to walk the pipe network, with a limit on the number of steps based on max uses left of the can. Why did we switch to just iterating over every direction? Also, I think this was causing the case where a + would not get fully colored.
|
|
||
| if (pipeTile.getNumConnections() == 2) { | ||
| int connections = pipeTile.getConnections(); | ||
| connections &= ~(1 << facing.getOpposite().getIndex()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You already have the facing of the spray, so why do you have to check over all other directions, instead of just the direction of the spray and its opposite?
# Conflicts: # src/main/java/gregtech/api/items/metaitem/MetaItem.java # src/main/java/gregtech/api/util/GTUtility.java
Fixed.
This isn't exclusive to the spray can. |
What
Implementation Details
Outcome
Spray cans are better to use.
Potential Compatibility Issues
Shouldn't be any, the durability based cans use the same NBT key so that should carry over.