-
Notifications
You must be signed in to change notification settings - Fork 25
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
Immersive Portals - Client crash fix #286
Conversation
…e_portals # Conflicts: # common/src/main/java/whocraft/tardis_refined/compat/portals/BOTIPortalEntity.java
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.
Looking good, some minor documentation needed
@@ -127,6 +127,14 @@ public boolean canCraftInDimensions(int width, int height) { | |||
|
|||
@Override | |||
public ItemStack getResultItem(RegistryAccess registryAccess) { | |||
if(result.type() instanceof ManipulatorItemResult manipulatorItemResult){ | |||
return manipulatorItemResult.recipeOutput(); |
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.
Why is this needed? I thought this only get used in vanilla crafting benches.
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're correct, but if for some weird reason a mod tries to render our recipes out, its better to render something than nothing
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.
Good catch, nice one.
@@ -79,4 +104,11 @@ public void setTardisId(UUID tardisId) { | |||
this.getEntityData().set(TARDIS_ID, Optional.of(tardisId)); | |||
} | |||
|
|||
public boolean isValid() { |
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.
Javadocs for what this does would be nice.
@@ -351,16 +355,29 @@ public static void destroyPortals(TardisLevelOperator operator) { | |||
if (portalEntry == null) { | |||
return; | |||
} | |||
|
|||
PortalManipulation.removeConnectedPortals(portalEntry.getInternalPortal(), portal -> { | |||
|
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.
Is this meant to be left empty?
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.
Yes
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.
LGTM
Fixes a issue where the game tries to attach a renderer to a entity that doesn't exist