-
-
Notifications
You must be signed in to change notification settings - Fork 362
Colony match updated and gradle properties #10928
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
Colony match updated and gradle properties #10928
Conversation
Colony match needs to take dimension ID for context. Also, gradle properties setup to run the required version of gradle.
@@ -1,6 +1,6 @@ | |||
distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-8.1.1-bin.zip |
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.
Whenever you change version of gradle wrapper please run twice gradlew wrapper --gradle-version [version]
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.
Ah that makes sense, I uh, definitely just wrote this in by hand which isn't the correct way to do it lol
edit: All done
JVM21 is only for the 1.21 branch. The main branch is still 1.20 with JVM17. |
This should be updated in the 1.20.1 tagged README then. I will update the README for this branch, so in the next tagged 1.20.1 patch update it will be more accurate. |
Updating gradle version properly (use gradlew wrapper instead of hard coding the version directly in the properties file)
…:Aaron9174/minecolonies into bug/allies-list-empty-on-diff-dimensions
…ug/allies-list-empty-on-diff-dimensions
All comments and merge conflicts addressed, looking for a re-review here |
@@ -377,7 +377,8 @@ public static void serializeNetworkData(@NotNull Colony colony, @NotNull Friendl | |||
{ | |||
for (final ColonyPlayer owner : colony.getPermissions().getPlayersByRank(colony.getPermissions().getRankOwner())) | |||
{ | |||
if (col.getPermissions().getRank(owner.getID()).isColonyManager() && col.getID() != colony.getID()) | |||
boolean colonyMatch = (col.getID() == colony.getID()) && (col.getDimension() == colony.getDimension()); |
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.
I would probably inline this as && (id != id || dim != dim), no?
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 got it chief, don't mind making the adjustment
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.
The change is in, but after re-visiting this, I think it was better before I made the change. As it was more readable via a variable name indicating there was a colonyMatch. Further, the boolean algebra was just simpler to understand before, even with context of these changes, I still had to whip out a truth table to make sure this logic was identical (which it is).
I don't readily see from this statement
col.getID() != colony.getID() || col.getDimID() != colony.getDimID()
that the current colony of the loop doesn't match the ID of the colony sent in the network request.
Truth table confirmed the logic is the same
@Nightenom Got Raycoms approval, mind taking a look at this when you get a chance? |
Was building locally wrong, just using the ./gradlew (without build), was just missing an parenthesis and was confused why it worked locally. Committing now. |
Missing a closing parenthesis
Colony match needs to take dimension ID for context. Colony IDs in different dimensions start at 1, so if you have two colonies in different dimensions trying to be allies, they won't show up in the list since the colony ID is not unique. Colony ID + dimension ID is unique and that's what this change involves.
Also, gradle properties setup to run the required version of gradle. Perhaps I have misconstrued the README directions, but I couldn't get this to run with JVM21, and instead got this working with JVM17 + a different gradle version. Possible that the gradle version change can be removed and only the ColonyView change is needed. Has been tested locally and on a server on version 1.20.1 at the most recent tag.
Closes #
Closes #
Changes proposed in this pull request
Testing
Review please