Carpet TIS Addition

Carpet TIS Addition

461k Downloads

Use both annotations at least

altrisi opened this issue · 2 comments

commented

Please. Don't use such hack to register rules in the new system. Please :'(

It is going to break given some of the references it uses are even scheduled to be removed or repackaged.

And according to the JVM spec a class having an annotation that isn't present at runtime is legal, so you should be able to just make a carpet.api.settings.Rule class that's not included in the jar and the jvm chooses the correct one at runtime by itself.

commented

Don't worry about future compatness when developing fabric carpet. I know those class are temporary, but at least they will exist for quite a while. I'll adapt when they get changed, just like how we do with Minecraft snapshots: we will make carpet (extensions) compact with Minecraft snapshots even those new Minecraft codes are not stable and will be changed in the future.

For the usage of reflections of ParsedRule and ParsedRule, the reason is that carpet doesn't provide an API to create / register a CarpetRule directly, without using the parsing @Rule annotation logic. That's why I chose reflection to save my kneecaps

For your suggestion, I don't really get that, wdym Use both annotations and make a class that's not included in the jar.

It would be really great if fabric carpet was able to finish the rule system refactoring in one go. The current state of faric carpet is in the somewhat a middle state of the old and the new system —— even fabric carpet itself still uses those deprecated codes.

And if fabric carpet provides enough flexible APIs for custom rules there's no more hack required. I don't want to write another whole CarpetRule implementation class when there is already one in carpet but you're not allowed to use.

commented

Will reopen this/another issue/a PR if I come back to a way to solve this without using this many internals (though I think it has changed a bit so it may not be using as many anymore). The idea about both annotations was that a classfile can have annotations that aren't present at runtime, so you could have both so they're detected in both old and new versions, given you now remap stuff (so you'd just call "the same" method). But I don't know the state of it right now, and it doesn't seem to have been causing issues.