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

[API] Add equality checks to all "Region" implementations #2673

Open
NonSwag opened this issue Dec 8, 2024 · 5 comments
Open

[API] Add equality checks to all "Region" implementations #2673

NonSwag opened this issue Dec 8, 2024 · 5 comments
Labels
status:pending Pending acceptance or closure. type:feature-request Request for something new

Comments

@NonSwag
Copy link

NonSwag commented Dec 8, 2024

The Problem

Currently, when comparing regions or putting them into a HashMap or sorting collections or basically anything that requires consistent equals and hashCode result

A Solution

Proper equals and hashCode methods for comparison of regions

Alternatives

Writing your own wrappers to add equals and hashCode methods

Anything Else?

No response

@octylFractal
Copy link
Member

You say "sorting collections" which implies you want them to implement Comparable too, but I think that is right out -- we don't have any consistent way to sort worlds really, that wouldn't just be an arbitrary sorting.

As for equals and hashCode, I suppose we could but the behavior might not be as you expect since you don't specify a specific use case here. Should two regions that cover all the same block points be the same, i.e. if you had a cuboid region and a poly2d region that covered all the same blocks, are they equal? Depending on your use-case the answer might be yes or no. How should world equality be considered? Should it be considered at all?

My current feeling is that since we have no use-case for it, we should not implement it at this time. But I'm open to discussion on it.

@NonSwag
Copy link
Author

NonSwag commented Dec 8, 2024

I was facing the issue of Map and Collection #contains returning false even if the region was inside the map or list
it would only return true if the exact instance of region was contained, but because I use cloned objects for immutability I can't check whether it already contains it or not

in my opinion, a simple comparison of the positions and region class is enough

as for the comparable I see your point and in that case I was simply able to implement my own comparing logic (in Stream#sort) so no real problem there

@octylFractal
Copy link
Member

There are issues with ignoring the World (technically incorrect, depending on what you're trying to do with the regions). It's possibly you may be better off using a record with the immutable properties you want, rather than cloning our regions everywhere.

@NonSwag
Copy link
Author

NonSwag commented Dec 12, 2024

In most cases if you want to compare regions you also want the worlds to be compared
If you don't you can just compare the points of the region
So my suggestion would be comparing all points and the worlds and obviously the type itself
I am not sure whether the world implementations have proper equality checks, depending on that it would be also required to add ones there

@wizjany
Copy link
Collaborator

wizjany commented Dec 12, 2024

WorldEdit barely uses the world. I think in the actual WE implementation itself, the World is only ever used for clamping bounds. Thus your idea of "most cases" is going to be entirely up to where you're getting those region objects and what you're using them for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:pending Pending acceptance or closure. type:feature-request Request for something new
Projects
None yet
Development

No branches or pull requests

3 participants