The 1.13/1.14/1.15/etc Port

[squeek502]

Figured I'd make an issue to coordinate/decide what all needs to happen for porting to the latest Forge version(s).

Things to decide:

• Is a 1.13 port worth it, or should AppleCore skip straight to 1.14?
  • Skipping to 1.14

Things to do:

• Figure out if anything has changed with how core mods interact with Forge
  • core mods now use javascript for ASM patching, example
• Update ASMHelper to work with 1.13/1.14 if necessary
  • Port of ASMHelper to Javascript in progress here
• Port the ASM patches:
  • BlockFood
  • ExhaustingActions
  • FoodEatingSpeed (also need to check if this is still necessary, but it probably is)
  • FoodStats
  • HungerHUD
  • PeacefulRegen
• Make sure that AppleCore incorporates/deals with any changes to vanilla mechanics in 1.13/1.14 (e.g. how health regen works, things like that). Check that constants in the API package still match the default values.
• Audit the API package to make sure the events still make sense, and to make sure AppleCore isn't duplicating anything new added to Forge
  • IEdible/IEdibleBlock still necessary?
  • ItemFoodProxy still necessary?
• Port the rest of the mod (should be pretty minimal outside of the ASM/API stuff)
  • Finish up all remaining TODO's

cc @primetoxinz @GirafiStudios @Cazsius

[squeek502]

Tried to get started with this, turns out probably the hardest part will be updating the build.gradle script for ASMHelper, as ForgeGradle and Gradle are much different now. Plus I completely forgot anything I knew back when I originally wrote that monstrosity, so it feels like I'm back to square one.

Maybe should look into a less hacky solution for including something like ASMHelper in a per-project manner.

EDIT: https://github.com/johnrengelman/shadow ?

[primetoxinz]

Frankly, you might not need ASMHelper. I'm not sure what the benefits of it were, but the new coremodding system in Forge is based in a sandboxed javascript loader.
If you are in need of assistance in porting I can help.

[primetoxinz]

Also, a 1.13 port is basically worthless. Forge has already moved on and it was not really stable during 1.13

[squeek502]

new coremodding system in Forge is based in a sandboxed javascript loader.

Whoops, didn't know that! Any docs for it? I was misled into thinking core mods hadn't changed by looking at a build.gradle of a 1.14 core mod and seeing FMLCorePlugin still in the manifest.

Looks like you're right that ASMHelper is worthless, or it would at least need a complete rewrite at this point.

If you are in need of assistance in porting I can help.

Pointing me in the right direction when I get stuck will help a lot. I haven't kept up with Forge changes at all and docs never seem to be easy to find.

[primetoxinz]

Unfortunately the docs on it are basically nonexistent.
The main repo for it has a scattered example, but it doesn't really explain anything: https://github.com/MinecraftForge/CoreMods
I managed to scrounge up the information from a bunch of mods that managed it somehow.
The basic requirements are as follows

- META-INF/coremods.json with a map of modid -> javascript file
This unfortunately means you can only have one .js file per

• After looking the coremod code I was actually wrong about this
  META-INF/coremods.json is a map of coremod name -> javascript file, a single mod can have as many different entries in this as they want.
• Put javascript anywhere in resources/, as long as the relative path is defined in your coremods.json
• You can't use nashorn's load functions to include additional js files
• Keep in mind that the java imports available in the sandbox are limited to the predefined list in net.minecraftforge.coremod.CoreModEngine

I used this as an example, it has a very simple coremod javascript file: https://github.com/TridentMC/FastBamboo
Mine is now signficantly more advanced than that example and might be more useful: https://gitlab.com/BetterWithMods/1.13/betterwithmods/blob/dev/projects/Core/src/main/resources/javascript/betterwithmods_core.js

If this doesn't really fit your style, it might be possible to register your own system for loading coremods but I imagine no one surrounding forge would be willing to give advice in that effort.

[squeek502]

Thanks for all the info @primetoxinz

Here's the branch I'm working in: https://github.com/squeek502/AppleCore/tree/1.14.3

[squeek502]

Ok, I think I finally have a solid path forward for this. Rewriting the transformers from scratch using Javascript sounds like a complete nightmare, so I think it's worth porting ASMHelper to Javascript so that the transformer code can be ported closer to 1:1. I've started porting ASMHelper here, and have set it up so that it's possible to port the ASMHelper tests as well to make sure the functionality stays the same. Here's the toMethodDescriptor test file for example:

var ASMAPI = Java.type("net.minecraftforge.coremod.api.ASMAPI");

function initializeCoreMod() {
	ASMAPI.loadFile('../../main/javascript/asmhelper.js');
	ASMAPI.loadFile('jankytest.js');

	assertEquals("(Linternal/class/name;Lclass/descriptor;)Lclass/name;", ASMHelper.toMethodDescriptor("class.name", "internal/class/name", "Lclass/descriptor;"));
	assertEquals("(FZ)V", ASMHelper.toMethodDescriptor("V", "F", "Z"));
}

See squeek502/ASMHelper@1d4a4e8 for some more context on how it's set up.

So the next step is getting ASMHelper ported fully to Javascript. After that, porting the AppleCore transformers to Javascript should be much easier.

[squeek502]

Stalled due to MinecraftForge/CoreMods#26

[James103]

@squeek502 Are you also porting to 1.15.2?

[squeek502]

Yeah, would probably just skip to 1.15 at this point. Still effectively blocked by MinecraftForge/CoreMods#26 though.

[James103]

Is it possible to port this to Fabric, or will it forever be Forge-only? A Fabric port may or may not work around the mentioned bug.

[squeek502]

Fabric port would definitely not have the same problem.

I'm not very involved in Minecraft modding anymore, and at this point am basically just updating my existing mods to newer versions, nothing more. I'm all for creating a Fabric version, but someone else would have to take the reins on that.

[Insane96]

Still effectively blocked by MinecraftForge/CoreMods#26 though.

That's sad, I was really looking forward to add lots of mechanics to Iguana Tweaks Reborn thanks to this

[Parker8283]

Hello friends, long time no see. A long time ago in a galaxy far far away, I made MinecraftForge/MinecraftForge#1758, which was the PR to lift the AppleCore changes up to Forge. However, I borked the PR and said I would fix it "tomorrow". Well, I guess tomorrow meant 5 years later, but now we are here! I am once again attempting to lift the core parts of the AppleCore API into Forge so that everyone can enjoy it and we can avoid the whole ASM problem altogether!

Currently, I have this branch that is my working branch. As of today, I think I have lifted the core parts and updated them to work better in the newer MC versions. I would love the feedback of the people who have helped to work on this mod and probably understand it better than I do. Just to make sure I don't miss anything or make some catastrophic errors. My goal is to make the PR to Forge by the end of this week. If anyone could spare any time and leave some comments for me, I would greatly appreciate it. I'm also in the SlimeKnights Discord if you wanted to do some closer-to-real-time chatter, do so in #progs-other-mods. Or you can DM me :)

Either way, just wanted to leave this here. Don't mean to pull the rug out from underneath this project, but I think our goal back in the day when squeek, prog, and I were updating Hunger Overhaul for 1.7 was to upstream these changes. Just never got around to it. Now with the new JS system, seems like a good time to try again!

[squeek502]

Looking good, thanks for doing this @Parker8283.

Some notes:

• One subtle thing that AppleCore does is fix the animation when eating speed is increased. Not sure if this is still the case, but in earlier versions, if a food item's use duration was increased, the eating animation would just sit frozen for a while and then only animate at the end at the normal rate (see here for an example of how to test this). AppleCore patches a few things to instead make the animation use the modified duration for the eating animation, so the animation is stretched across the entire duration instead of being frozen for most of it. It's been a long time since I touched this code, so I don't remember the exact details here.
• I imagine IEdible might be questioned. That part of AppleCore never felt quite right, and a better solution is probably possible. Just something to keep in mind. The problem it was attempting to solve was the case of block-based foods like cakes, which have never really meshed well with AppleCore's implementation.
• The example mod code would be a decent starting place for testing all the various capabilities of the API.
• Definitely worth looking at the api_impl package to look for various helper methods that might be worth adding to Forge somewhere (see IAppleCoreAccessor and IAppleCoreMutator for documentation)

[Parker8283]

Hey squeek, thanks for the feedback!

• Interesting, does this effect all items that have their use duration changed? If so, I think that is probably better as a separate PR. I can give it a test later to verify.
• I could see it. I'll roll with it for now and if I get questioned, I'll ask what a better system might look like.
• Awesome, thank you! I'll incorporate them into the tests required for Forge PRs
• So I have already included most of what was in IAppleCoreAccessor in different parts of the code (mostly ForgeEventFactory). As for everything else, I might have to reevaluate what I can do. From what I remember, Forge doesn't like just random utility classes. Though maybe that kind of thing has changed over the past 5 years. I'll look at it again tomorrow and see what I come up with.

Just pushed my update to my branch. Mostly documentation, test mods, and small fixes. Hopefully a PR will be made soon! Thanks again for your help.

[squeek502]

Sounds good.

From what I remember, it only affected the eating/drinking animations for some reason, but I could be wrong about that. Or, perhaps the bug affects all items but AppleCore intentionally only affects the eating/drinking animations. I'd have to look at the code again to understand what exactly the patches are doing. Will get back to you on that. I agree that it would be better as a separate PR, though.

[squeek502]

Ok, details on the animation stuff:

• Forge adds a hook for ForgeEventFactory.onItemUseStart in LivingEntity.setActiveHand and then uses the returned (modified) duration to set the field LivingEntity.activeItemStackUseCount
• However, LivingEntity.getItemInUseMaxCount does not use the modified duration for its max, and instead queries activeItemStack.getUseDuration() directly
• This means that the duration will count down from the correct start point, but the animation will not be aware of the 'real' max, and treat the default as the max instead. So, when animating, it will just freeze until the count gets within the range of the normal duration.
• To fix this, AppleCore added a new field to LivingEntity to store the modified max duration (set to the modified duration at the same time as LivingEntity.activeItemStackUseCount), and then patches LivingEntity.getItemInUseMaxCount to return the correct max specifically for EAT/DRINK:

  AppleCore/java/squeek/applecore/asm/Hooks.java

  Lines 155 to 162 in 6445352

  	public static int getItemInUseMaxCount(EntityLivingBase entityLiving, int savedMaxDuration)
  	{
  	EnumAction useAction = entityLiving.getActiveItemStack().getItemUseAction();
  	if (useAction == EnumAction.EAT || useAction == EnumAction.DRINK)
  	return savedMaxDuration;
  	else
  	return entityLiving.getActiveItemStack().getMaxItemUseDuration();
  	}

So this could be a fix applied to items in general, and it seems like it almost certainly affects 1.16.x still. I personally would consider it a Forge bug.

[Parker8283]

Awesome, thanks for this research. I will look into this and make a PR to Forge to fix that.

In other news, MinecraftForge/MinecraftForge#7266 has been made! This should be the bulk of what AppleCore was. Thanks for your help squeek! Now starts the review process...hopefully they don't tear too much into it :)

[Parker8283]

Over one year later, just wanted to update here and say that I haven't forgotten about this. Just made MinecraftForge/MinecraftForge#8405, which is the 1.18 version of the original PR (it's been so long that a new minecraft version came out in the meantime!), and I will do my best to stay active on this PR on GitHub and in the Forge Discord (seems to be the way to do things now) so that it can finally be merged. I am still hoping for MinecraftForge/MinecraftForge#7266 to be pulled as well for 1.16, but since that is now the LTS version, it would be after the new 1.18 PR is pulled. Hopefully we can get that done soon!

[squeek502]

Thanks for the update, hope it can get merged.

[Shadows-of-Fire]

I'm attempting to move the forge PR forward.
It will still have to go through Lex's approval once it is deemed assignable, and that may be a hurdle on it's own.

I'm asking anyone who has an interest in the state of the system go review the PR for anything that could be done differently or better, as you guys in this thread are likely more knowledgeable about what you want out of the system than I am.
Specifically the 1.18 variant MinecraftForge/MinecraftForge#8405 - this has to get merged first before the 1.16 version can be considered.

[Parker8283]

Hey folks, minor hiccup on the Forge PR. The Forge team has requested some core design changes, so I will have to look into that. See this and above comments on the PR to see what I mean. Since this will require some non-trivial changes, I'm hoping I can get the input of some people that use the current API as it stands on what the best path forwards is for Forge's requests. I just want to make sure that my eventual changes don't suddenly make some use case impossible or really cumbersome in the "new" API. If you have input or ideas or just want to link how you use this so I can better understand what all is needed from this system, please leave it here, I would greatly appreciate it. I'll try to find some time to go over everything within the next week or so and hopefully we can get this back on track relatively quickly.

[squeek502]

In terms of why AppleCore exists, here's the primary use case it was created to support:

• Hunger Overhaul being able to modify how much hunger/saturation foods give
• The Spice of Life being able to modify how much hunger/saturation foods give per-player
• Hunger Overhaul and The Spice of Life being compatible with eachother

And secondarily:

• Hunger Overhaul being able to modify how/when exhaustion happens
• Hunger Overhaul being able to modify how/when health regen happens
• Hunger Overhaul being able to modify how/when starvation happens

Before AppleCore existed (in 1.6.4), Hunger Overhaul overrode FoodStats and unconditionally set the player's FoodStats to HO's custom implementation on player spawn--a technique that only works for one mod at a time. With AppleCore in 1.7.10, both mods were able to go through AppleCore's API instead, and were therefore compatible without needing any special casing.

[Shadows-of-Fire]

I have a fair bit of knowledge with the attribute system, I'll poke around and see what is appropriate to be an attribute.
I think each exhaustion action will need to be an event still, but things like max exhaustion, hunger, sat, can be attributes just from memory.

Not everything can be widened to LivingEntity, but what can be will need to be. Since lex un-assigned it, it means there was some back-channel comms about that being necessary.

[James103]

Don't forget to take into account that Scaling Feast by @yeelp does the following:

• Modifies the player's max hunger via a new item and a new attribute (modifier)
• Adds an enchantment, potion effect, and attribute that changes the amount of exhaustion players receive
• Adds a block that restores a percentage of the player's max hunger
• Adds an enchantment that makes food more nourishing to the wearer
• Adds a curse that changes the amount of exhaustion needed to lose a food point
• ... and more (see the mod repo and its wiki)

[Parker8283]

Hey friends, haven't forgotten about this! I'm making pretty good progress on updating the Forge PR to meet the Forge Team's requests. One thing that was brought up on the PR was the tick period events, and questioning their necessity. You can read the comment here. It seems like these events have existed since the beginning of AppleCore, so I'm not sure who needed their use, and if just keeping the tick period the same and scaling the actual changes done would be acceptable. I'm on the fence personally on that, since I can see the value from having a mod that wanted to make more frequent changes over really big less-frequent changes, would probably give the player a bit more context. Downside, that's three more events that get fired pretty often, and those aren't cheap (although more realistically, they would be moved to entity attributes, which are definitely cheaper). Just looking for some perspective and/or context for the original versions so I can make sure I understand everything before making a decision.

On top of that, another request was to roll things like AllowExhaustion and Exhausted into one event if possible. There are a few places where we have an Allow* event, very closely followed by the actual event we are allowing. To me, I can see those potentially being rolled into one and not losing functionality, but I just want to double-check here before I do that, to make sure I'm not missing something.

I think those are the only two things left to resolve for me before I update the PR and hope it's good to go now. Hopefully we get this upstreamed soon! A note that because 1.19 seems to be not too far off, that will become the new main target probably, and then it can get backported to 1.18. It would then seem that 1.16 will be left behind. Such is the endless march of new Minecraft versions :)

[Shadows-of-Fire]

You could make the tick period an attribute that can be modified instead of an event, maintaining both performance and functionality.

The allow events can probably be safely rolled into the real event imo.

[squeek502]

@Parker8283 For the tick period stuff, GetRegenTickPeriod is used in Hunger Overhaul. Unsure if it could be converted to something that doesn't allow for modification of the tick period. The others were likely just for the sake of completion, without any specific use-case in mind.

Rolling the Allow* stuff into the real event seems potentially okay, but hard to know exactly:

• Here's a use-case from Hunger Overhaul that can be replicated to check that it would still be possible.
• AllowSaturatedRegen and AllowRegen are tied together in a somewhat complicated way that might be tough to replicate without the Allow events (but also might be unnecessary to replicate)

[yeelp]

Don't forget to take into account that Scaling Feast by @yeelp does the following:

• Modifies the player's max hunger via a new item and a new attribute (modifier)
• Adds an enchantment, potion effect, and attribute that changes the amount of exhaustion players receive
• Adds a block that restores a percentage of the player's max hunger
• Adds an enchantment that makes food more nourishing to the wearer
• Adds a curse that changes the amount of exhaustion needed to lose a food point
• ... and more (see the mod repo and its wiki)

I'm a little late to the party (read: very). A lot of Scaling Feast was me trying to use the AppleCore API in unique ways. I've gone ahead and documented which AppleCore events I use, and for what purposes. Hopefully that helps outlining use cases. Most of these event listeners are in features and handlers if you want to see the use cases.

FoodEvent:
FoodStatsAddition: Allowing a hunger overflow system, and a dynamic starvation system.

HealthRegenEvent:
AllowRegen: extending the health regen to a dynamic hunger bar size
AllowSaturatedRegen: Same as above.

StarvationEvent:
Starve: implementing a dynamic starvation system, and preventing starvation with a hunger Absorption effect.

ExhaustionEvent:
ExhaustionAddition: implementing a dynamic starvation system, modifying exhaustion rate based on attribute value.
Exhausted: For calculations involving a hunger version of the Absorption effect.
GetExhaustionCap: This is a big one. Minecraft hard caps maximum exhaustion. Not sure why. This was a request by me to allow this cap to be variable. Check here for details. I can see it being reasonable to just remove the cap entirely instead of keeping the event, since it feels very arbitrary to have the cap in the first place.

HungerEvent:
GetMaxHunger: Setting max hunger.

That's about everything I can find for AppleCore events Finding the individual AppleCoreAPI calls is a little more involved but is also used quite heavily. Pretty much everything mentioned except the HealthRegenEvents are needed for Scaling Feast.

[Sinhika]

Use case I desperately want available again: Hunger in Peace.
I play on peaceful mode for reasons, but I really want food to matter.