Skip to content

Conversation

@benhunter
Copy link
Contributor

No description provided.

Hanmac
Hanmac previously approved these changes Jan 22, 2026
@Jetz72
Copy link
Contributor

Jetz72 commented Jan 23, 2026

Could simplify it even further with an Objects.equals.

@Hanmac
Copy link
Contributor

Hanmac commented Jan 23, 2026

Also, Objects.hash for the hashCode function.
(but i can do that later)

@benhunter benhunter force-pushed the cleanup-lobbyplayer branch from e613499 to 350175c Compare January 25, 2026 03:36
@benhunter
Copy link
Contributor Author

Could simplify it even further with an Objects.equals.

Great point - I removed the override altogether. Tests pass and game runs as expected.

@benhunter benhunter requested a review from Hanmac January 25, 2026 05:03
@Hanmac
Copy link
Contributor

Hanmac commented Jan 25, 2026

Could simplify it even further with an Objects.equals.

Great point - I removed the override altogether. Tests pass and game runs as expected.

No, we meant use Objects.equals

For Objects.equals(this.name, other.name)

@benhunter
Copy link
Contributor Author

Could simplify it even further with an Objects.equals.

Great point - I removed the override altogether. Tests pass and game runs as expected.

No, we meant use Objects.equals

For Objects.equals(this.name, other.name)

Right, I see the difference. IntelliJ warned about requiring a class check, so the code would look something like this:

    @Override
    public boolean equals(Object obj) {
        if (obj.getClass() != this.getClass()) {
            return false;
        }
        return Objects.equals(this, obj);
    }

Does this behave differently (for our purpose) from allowing Object.equals() to run without override?

@Hanmac
Copy link
Contributor

Hanmac commented Jan 25, 2026

The difference is avatarIndex and the other properties should be ignored

@benhunter
Copy link
Contributor Author

The difference is avatarIndex and the other properties should be ignored

I think it's there now :)

@Hanmac Hanmac merged commit 3f7f783 into Card-Forge:master Jan 26, 2026
2 checks passed
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