crash-2021-07-21_11.53.06-client.txt

I got this error when trying to load a bunch of mods together, so it could be possible one of them isn't working with this mod. It might be a problem with someone else's mod, but I hope this crashlog helps

Fixed in 0.2.2

This protest does not affect Forge and only affects end users of the mod. Perhaps a more effective form of protest would suffice?

Is there any reason why isPresent() cannot be used?

if (ConjuringForgery.SOUL_ALLOY_SWORD.isPresent() && item != ConjuringForgery.SOUL_ALLOY_SWORD.get())

I must protest the solution you have implemented for this problem.

Why not simply check .isPresent() in your mixin? Are you aware that the getAttributeModifiers() mixin is called for every ItemStack and not only for ItemStack containing the soul alloy sword item?

Calling Unsafe because it is "funnie" seems a poor design choice.

Yes, of course it's a poor design choice. Just like not giving me the ability to simply use something like getOrNull(). You see, this is a minecraft mod, I had fun implementing this as a sort of silent protest against the way Forge does things and that is what's important to me at the end of the day.

I use Java 16 and Forge 1.16.5 myself, so this change would disqualify me from using your mod.

I would prefer not to be in the position to choose between using your mod and using all of the other mods that I use for 1.16.5, but if you are not willing to change your mind, that is the position I will be in.

I also do not particularly agree with your mindset regarding poor code as that attitude is what ultimately leads to the tragedy-of-the-commons you claim you are protesting against. The question you are posing is: "Should I write good code if everybody else writes bad code?" As a user of your software, I would hope that your answer is yes.

isPresent() would actually be massively blowing up the code, a far better solution would be to simply use orElse(null). It's just that the performance impact the current solution has is so minimal compared to everything else that happens in Minecraft that I deem it more than justified to overengineer the shit out of this for kicks and giggles.

Oh and just because it's incredibly funny as well, this is legitimate code I had a whole lot of fun writing a few weeks ago that resides yet undiscovered in here https://github.com/LordDeatHunter/Alloy-Forgery/blob/d82c20967e66dba7468f4aa5ac38cee7bdefab61/src/main/java/wraith/alloyforgery/recipe/AlloyForgeRecipe.java#L121

@Override
public String toString() {
        var outString = String.valueOf(lowerBound);
        var chars = outString.chars().mapToObj(value -> (char) value).collect(Collectors.toList());

        if (upperBound != lowerBound) {
            if (upperBound == -1) {
                chars.add('+');
            } else {
                var to = " to ".chars().mapToObj(value -> (char)value).collect(Collectors.toList());
                to.forEach(character -> chars.add(character));

                var bound = String.valueOf(upperBound).chars().mapToObj(value -> (char)value).collect(Collectors.toList());
                bound.forEach(character -> chars.add(character));
           }
        }

        var output = new StringBuilder();
        chars.forEach(character -> output.append(character));

        return output.toString();
    }

}

Before I waste any more of your time, is there any chance that you will change your mind on this?

I am a big fan of the mod and greatly appreciate the time you have put into it so far, which is why I am disappointed to see this attitude.

I can see that personal amusement is perhaps more important to you than feedback. I do not understand how using isPresent() in one location would blow up the code more so than the solution you posed, but if it was implemented this way solely for your amusement I won't attempt to take that from you.

Ok, just to fully explain my sentiment here. We're talking about a mod for a game that has very questionable code all over the place using a modloader that's just as concerning and outdated in implementation. I personally do not see any downsides to using Unsafe for this apart from not being Java 16 compatible, but Forge itself isn't. Of course there are way cleaner solutions, but this one is not really inferior to those apart from in verbosity, which I personally also don't really see as significant.

I also have to use this is quite a few locations because I have more mixins that could in theory be called before my Items are registered, meaning that using isPresent() would add a significant amount of unnecessary boilerplate that can be avoided by just using orElse(null) because that will also make the comparison fail and thereby achieve my desired result.

And to answer you question, unless another issue crops up because of this, no I will not change the implementation, at least not until the next version.

Ok, now I am genuinely curious. How does one run Forge on Java 16?

And then be immediately faced by this error because Forge isnt Java 16 compliant

Also, regarding my sentiment on code quality. Of course I always strive for the best code I can reasonably produce, especially if it's in a place that I expect other developers to work with. This, however, is an internal part of the mod that no one, not even a developer trying to add some kind of compatibility, will ever get to see. This, and the fact that the code isn't bad but just inappropriate, I believe makes it ok to include this.

If you have Java 16 installed, you can select the JVM binary from your launcher of choice.

I do not know what to say other than that you are mistaken about your assumption. There is currently a green banner at the top of the Forge forums that describes how to run Forge on Java 16, and I have had no issues thus far.

Either way, I have made my case and I can see I will not be able to change your mind. I acknowledge your claim to always strive for code quality but find it somewhat dubious in light of the rest of this discussion.

I would suggest that if personal amusement is your primary object or if you find developing for Forge particularly onerous, to simply drop support rather than engage in self-sabotage.

I hope your weekend continues to be enjoyable.

So just to clarify my point. Forge does not officially support Java 16 and thereby I don't have to either, they even tell you that you shouldn't use it. The workaround in the forums is just as much a hack as is using Unsafe to get my RegistryObject value and in fact the Unsafe problem could be solved using extremely similar JVM options but instead for the sun.misc package. In any case, I will now lock this as I don't see any useful discussion emerging further.

I have just run into this issue as well, and I believe I have identified the probable cause.

On Line 40 of ItemStackMixin.java a call is made to the get() method of RegistryObject without regard to whether the RegistryObject contains a value.

If the method that this mixin is modifying is called before the Soul Alloy Sword is registered, calling get() will crash because internally RegistryObject calls Objects.requireNonNull() here. According to the documentation of the get() method (visible a few lines above), it is advised to check for isPresent() before calling get().

There are valid reasons why the mixin-targeted method getAttributeModifiers() could be called before the item is registered, and given that Forge mod loading is mostly parallelized, it is probably risky to call get() on a specific RegistryObject in a mixin without knowing whether the underlying instance has actually been registered.