Conversation
| } else { | ||
| String version = Bukkit.getVersion(); | ||
| Matcher matcher = Pattern.compile("MC: \\d\\.(\\d+)").matcher(version); | ||
| Matcher matcher = Pattern.compile("MC: (\\d+)\\.(\\d+)").matcher(version); |
There was a problem hiding this comment.
Would it not be far simpler to just modify this expression a bit? For example:
Matcher matcher = Pattern.compile("MC: (?:1\\.)?(\\d+)").matcher(version);Test it here: https://regex101.com/r/5tSuU8/1
This way, none of the existing uses of supports need to change, and new ones can simply use supports(26) without considering the way version numbers are written.
There was a problem hiding this comment.
Yes, that would be enough to check for version 26.1, but afaik the next big release is supposed to be 26.2. With your proposal there would be no way to differentiate between 26.1 and 26.2.
There was a problem hiding this comment.
What I would recommend is to not try to parse each digit, but to instead parse out the entire version string, and turn it into a list of ints. Finally if the first int is a 1, remove it.
This is what I do for my plugin, (note this is in kotlin)
mcVersion =
Regex("""MC:\s*([\d.]+)""")
.find(plugin.server.version)
?.groupValues
?.get(1)
?.split('.')
?.asSequence()
?.mapNotNull { it.toUIntOrNull() }
?.let { seq -> if (seq.firstOrNull() == 1u) seq.drop(1) else seq }
?.toList() ?: emptyList()Then make the supports function be varadic over integers. Since 26 is greater than 21 (from 1.21), this works.
There was a problem hiding this comment.
Yes, that would be enough to check for version
26.1, but afaik the next big release is supposed to be26.2. With your proposal there would be no way to differentiate between26.1and26.2.
Sure, but it doesn't look like you've used supports(26, 1) here yet, so who knows if there will be a need to distinguish them. 1.21.9 had a pretty significant number of internal changes iirc, but they don't need to be distinguished here except in documentation.
Even if 26.2 does end up needing to be distinguished in code, I would still advocate for dropping the 1. prefix here as it contains no real meaning, and simply matching on the other two numbers if required, e.g.:
Matcher matcher = Pattern.compile("MC: (?:1\\.)?(\\d+)(?:\\.(\\d+))?").matcher(version);
if (matcher.find()) {
VERSION_MAJOR = Integer.parseInt(matcher.group(1));
VERSION_MINOR = matcher.group(2) == null ? 0 : Integer.parseInt(matcher.group(2));
}Test: https://regex101.com/r/5tSuU8/2
What I would recommend is to not try to parse each digit, but to instead parse out the entire version string, and turn it into a list of ints. Finally if the first int is a 1, remove it.
Sure, that would work as well
There was a problem hiding this comment.
Sure, but it doesn't look like you've used
supports(26, 1)here yet, so who knows if there will be a need to distinguish them
While it might not be needed for 26.1 or maybe even 26.2, as this will be the new versioning system going forward I would assume that it would be required at some point.
Both of you make some good points, but I'll wait for feedback from the maintainer before making changes.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
|
Thank you for your work! Am hosting it on my maven repo all while waiting for it to get merged. <repository>
<id>marcely-repo</id>
<url>https://repo.marcely.de/repository/maven-public/</url>
</repository>
<dependency>
<groupId>com.github.cryptomorin</groupId>
<artifactId>XSeries-Fork</artifactId>
<version>13.6.0</version>
</dependency> |
|
I have published a build that contains the changes from this PR as well as the fixes from #403 and #404 to maven central. Still hoping to get this merged tho See https://central.sonatype.com/artifact/io.github.almighty-satan/XSeries <dependency>
<groupId>io.github.almighty-satan</groupId>
<artifactId>XSeries</artifactId>
<version>13.6.0+26.1</version>
</dependency>Source can be viewed here: https://github.com/Almighty-Satan/XSeries/releases/tag/v13.6.0%2B26.1 |
[21:59:53 ERROR]: Error occurred while enabling ItemRecycler v1.5.3 (Is it up to date?) |
|
@ImYenil Your version check is wrong.
|
|
Thanks |
|
Hey guys, everything is to good to get this merged or? |
|
😶🌫️ |
7 participants
This adds the new materials, sounds and particles as well as support for Minecrafts new versioning system (XMaterial would previously throw an
IllegalArgumentExceptionin the class initializer)Some stuff is probably still missing but this at least gets my plugin to initialize.
Issue: #399
If you are someone who can't wait for official updates and needs this right now, you can temporarily use this: https://central.sonatype.com/artifact/io.github.almighty-satan/XSeries (also includes the fixes from #403 and #404)