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

Immersive Portals - Client crash fix #286

Merged
merged 6 commits into from
May 24, 2024
Merged

Conversation

Jeryn99
Copy link
Collaborator

@Jeryn99 Jeryn99 commented May 24, 2024

Fixes a issue where the game tries to attach a renderer to a entity that doesn't exist

Copy link
Member

@50ap5ud5 50ap5ud5 left a 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();
Copy link
Member

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.

Copy link
Collaborator Author

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

Copy link
Member

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() {
Copy link
Member

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 -> {

Copy link
Member

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

@50ap5ud5 50ap5ud5 left a comment

Choose a reason for hiding this comment

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

LGTM

@Jeryn99 Jeryn99 merged commit d7b4144 into minecraft/1.20 May 24, 2024
@Jeryn99 Jeryn99 deleted the immersive_portals branch May 24, 2024 10:34
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.

3 participants