Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Some properties cannot be accessed from block states #19

Open
jellysquid3 opened this issue Jan 31, 2021 · 7 comments · May be fixed by #22
Open

Some properties cannot be accessed from block states #19

jellysquid3 opened this issue Jan 31, 2021 · 7 comments · May be fixed by #22
Assignees
Labels
bug Something isn't working

Comments

@jellysquid3
Copy link
Member

This appears to be wrecking havoc on some mods that share properties between different block states.

@jellysquid3 jellysquid3 added the bug Something isn't working label Jan 31, 2021
@jellysquid3 jellysquid3 self-assigned this Jan 31, 2021
@jellysquid3 jellysquid3 added this to the Version 0.3 milestone Jan 31, 2021
@coderbot16
Copy link

I think I've figured out what could be the cause:

Terrestria uses the base library Terraform, which has a class for defining small log blocks (such as sakura logs). This class creates its own WATERLOGGED property:

https://github.com/TerraformersMC/Terraform/blob/844a986daf93e908202d823cc6f733d5909a861d/terraform-wood-api-v1/src/main/java/com/terraformersmc/terraform/wood/block/BareSmallLogBlock.java#L50

	public static final BooleanProperty WATERLOGGED = BooleanProperty.of("waterlogged");

Thus, if you do state.get(Properties.WATERLOGGED), you will get into a situation where there exists a waterlogged property in the state where waterlogged != Properties.WATERLOGGED but waterlogged.equals(Properties.WATERLOGGED) == true.

HydrogenImmutableReferenceHashMap only uses reference equality (==), not the equals method (.equals()), so it will fail to find this special waterlogged property, but in vanilla, the equals method is used, so it works.

The question then becomes: whose fault is this? Clearly, Terrestria / Terraform should just use Properties.WATERLOGGED if they are going to be accessing the property through Properties.WATERLOGGED instead of creating their own property.

However, given that this occurs at the very least with a few mods, Hydrogen has a few options:

  1. Use .equals() for a slight performance hit. Since most implementations of .equals() check reference equality first, this shouldn't cause too many problems.
  2. Warn when mods try to construct properties that are equivalent to existing properties through .equals() but not through reference equality
  3. Do nothing and wait for those mods to fix the problems on their side.

@coderbot16
Copy link

It looks like BetterEnd will be fixing this in a future release: paulevsGitch/BetterEnd@67a4b19

@coderbot16
Copy link

As well as BetterNether: paulevsGitch/BetterNether@40ce83e

I will try to ship a fix for the Terraformers' mods soon.

@coderbot16
Copy link

This has been fixed on Terrestria's side in Terrestria 2.2.0, however BetterEnd and BetterNether do not have fixed versions yet.

@magneticflux-
Copy link

It might be possible to "intern" or cache calls to BooleanProperty.of like String.intern() does, so separate calls to BooleanProperty.of(x) return the same object. That would guarantee the uniqueness of Properties at all times, and allow reference equality to be used without extra care taken by other mods.

@Binero
Copy link

Binero commented Feb 11, 2021

Sandwichable also has a similar issue.

https://paste.ee/p/HhwLq

@magneticflux-
Copy link

Sandwichable also has a similar issue.

https://paste.ee/p/HhwLq

I just tested with #22 and it seems to fix it.

Binero added a commit to Binero/Sandwichable that referenced this issue Feb 11, 2021
Binero added a commit to Binero/Sandwichable that referenced this issue Feb 11, 2021
Binero added a commit to Binero/Sandwichable that referenced this issue Feb 11, 2021
@jellysquid3 jellysquid3 removed this from the Version 0.3 milestone Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants