Skip to content
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

Nutrition API #74

Closed
Wabbit0101 opened this issue Apr 24, 2018 · 42 comments
Closed

Nutrition API #74

Wabbit0101 opened this issue Apr 24, 2018 · 42 comments
Labels
enhancement A feature request or expansion

Comments

@Wabbit0101
Copy link

Lovely mod, but there doesn't seem to be a way (config or api-wise) to tell nutrition about foods that represent an adhoc composition of other foods.

Example: assume my mod has a sandwich maker...the player can put any combination of a set of items for which Nutrition has the static food item values (preconfigured or customized), but there isn't a way for me at game time to tell the mod "hey, this particular Itemstack of 'sandwichItem' contains a potato bread, steak, fried onions, and a roasted carrot" (There is no useful preconfigured nutrition stat for 'sandwichItem' as it's technically "empty" of value until the player makes a particular one.)

So this is an enhancement request for an API for such a thing: perhaps an Interface or Capability? Or is this somehow currently possible and I've just missed something?

@WesCook
Copy link
Owner

WesCook commented Apr 29, 2018

Hi @Wabbit0101,

You're right that this currently isn't possible. The current design of the mod is to only supported ItemFood entries, with special support for BlockCake and ItemBucketMilk as well. I've given some thought to supporting more complex food types like this especially in regards to an API, but don't currently have plans for implementing this functionality. It may be the goal of the next large update, if my coding chops are up to it.

For now though I'm going to mark this as closed as it isn't what I'm focused on right now. I appreciate the suggestions though, and the useful example case above. I'll make note of them for the future.

Thank you.

@WesCook WesCook added the enhancement A feature request or expansion label Apr 29, 2018
@WesCook WesCook closed this as completed Apr 29, 2018
@Snownee
Copy link

Snownee commented Jun 20, 2018

So what if I have a ItemFood with special NBT to tell every nutrient's increment? Would you like to add this feature or are you willing to merge a PR about this?

@WesCook
Copy link
Owner

WesCook commented Jun 20, 2018

Hi @Snownee,

By "nutrient's increment", are you referring to separate granular nutrient values (eg. 3 fruit, 2 grain)?

I've thought about how this might work. I think it would be possible, but would require more serious underlying changes in how the mod calculates nutrition. More importantly, it also changes how the user understands and interacts with the mod.

I think I'd like to avoid this. I understand the desire for having granular nutrient amounts, but the mod was designed around a simpler premise, and I like the level of realism it's currently at.

The JSONs were also written with this in mind, and would become largely inaccurate or "fuzzy" if granularity were introduced.


That said, I have given some thought to the problem proposed in the original post about how to support more complex foods (such as modular sandwiches).

There's many options here: JSON, CraftTweaker plugin, OreDict, NBT, IMC.

Some considerations:

  1. Does the implementation method require the item be an ItemFood? If not, then a way to specify the nutritional value is required.
  2. Is it easily editable by players, modpack authors, and/or modders?
  3. If editable by modders (eg. by IMC), how do we support customized nutrients? Not all modpacks will use the default 5 food group system (this is by design).

I doubt any solution satisfies all considerations, but I'd like to get as close as possible without inheriting too much technical tech.

Advanced JSON

One idea I discussed previously was advanced JSON formats that allow specifying nutrient values in the JSON. This would allow very easy editing for users/modpack authors, though exclude modder support. It also carries the implications of granular nutrients, as discussed above.

OreDict

Another thought I had was to piggyback off the existing oredict support by introducing nutrient-based oredict entries. A nutrient-named "vegetable" would automatically get support for nutrientVegetable. If a modder wanted to, they could then very easily add nutrient support by adding an oredict entry to their foods. This is nice because it's supported fairly easily by modders and modpack authors (via CraftTweaker). Support for complex foods still depends on it being an ItemFood with a heal value, however. Realistically I don't know how common that is for complex foods.

NBT

I hadn't thought about using NBT before. It carries a lot of the same implications of using OreDict, but is a bit less visible to players (won't show in advanced tooltips by default). Also editable using CraftTweaker, though support isn't quite as good in other tools. The one big advantage of NBT over OreDict is that it's more structured, and would allow a nutrition value to be specified alongside nutrients.

I'd be interested to hear your thoughts on some of these considerations, Snownee. As said I doubt there's a perfect solution, but it's worth weighing all the options.

Cheers!

@Snownee
Copy link

Snownee commented Jun 20, 2018

@WesCook Thanks for your reply!

I am making such a "modular sandwiches" mod, and trying to find a way to support Nutrition. That mod is aiming to let players creating NBT-based modular foods. (It works similar to Tinker's Construct) So the JSON and OreDict design is impossible to meet this need.

The previous tooltips will still nicely work. See Tinker's Construct, the tooltips are fully NBT-based. Think about things like this:

In ca.wescook.nutrition.nutrients.NutrientUtils

// Returns list of nutrients that food belongs to
public static List<Nutrient> getFoodNutrients(ItemStack eatingFood) {
    // do something
    if (stack.hasCapability(NUTRIENTS_HANDLER_CAPABILITY, null)) {
        return stack.getCapability(NUTRIENTS_HANDLER_CAPABILITY, null).getFoodNutrients();
    }
    else
    {
        // do the previous logic
    }
}

It can be very friendly.

EDIT: For your first consideration, I don't think it has to be. Just implement a blank food for customization is okay. And you may be interested in the implement of Forge energy (net.minecraftforge.energy).

Sorry for my poor English, I hope I wouldn't cause too many confusions.

@WesCook
Copy link
Owner

WesCook commented Jun 21, 2018

Thanks for your response.

I understand that JSON would not suit your use case. However I'm curious why OreDicts would not be possible if NBT tags are? Would it not be possible to attach the oredict dynamically? (eg. nutritionGrain when adding bread). Or is that too hacky?

I understand there's some confusion over if oreDict'd items are actually equivalent or not. Mojang's tag system in 1.13 might be more appropriate.

Assuming NBT were the best approach, would you mind giving an example of how the NBT might look so that Nutrition could read it? I'd like to keep it generic enough that I don't need to add mod-specific support in code. Were you thinking something like this?

{
    "nutrition": 1.5F,
    "nutrients": [
        "grain",
        "vegetable"
    ]
}

Or maybe wrapped in another container to avoid contaminating the root namespace...

And you may be interested in the implement of Forge energy

You know, I recently completed a full refactor of the Capabilities system and I still find capabilities completely confusing. Somehow I can't wrap my head around them.

@Wabbit0101
Copy link
Author

👍 for the NBT or 🥇 for the capability(api) approach if you're taking a poll. I'm not sure how the ore dict approach would work without the mod author still telling Nutrition about the composition of the sandwich (ie the list of dictionary keys in the specific sandwich itemstack). I like your NBT example above but maybe use a single composite at top-level "Nutrition" for collision insurance, like:

{
    "nutrition": {
      "value": 1.5F,
      "nutrients": [
          "grain",
          "vegetable"
      ]
}

The current Nutrition capability api would need tweaking to return a structure and not a list of nutrients so the total food value could be passed back and future expansion would be supported. Capabilities are nice as it frees Nutrition from oredict/nbt maintanance -- only support your capability and INutritionProvider interface (of which you've got 95% already). It also avoids the whole ItemFood vs Item vs IEdible (applecore) thing. I also don't grok capabilities fully but when it works it works really well (like for fluids...itemhandler less so).

With regards to NBT, after some looking at 1.13 I think NBT support will have to get mucho better all around with the great flattening and jsonification of everything. Even for vanilla things like enchantments and potions, it a royal pain currently in JSON-based data.

@Snownee
Copy link

Snownee commented Jun 21, 2018

You know OreDicts are based on Item and metadata which means I have to maintain a magical list of ItemStacks for every single case looks like:

food1:
OreDicts:
nutritionValue1
nutritionGrain
nutritionVegetable

food2:
OreDicts:
...

I think the OreDicts could be a way, but it shouldn't be the way. Moreover it could be a more serious headache for someone who want to change a single food's nutrient to another.

And I just have a new idea: is it a chance to make each nutrient has different value? Maybe it looks like:

{
  "nutrients": {
    "grain": 1.5F,
    "vegetable": 1.0F
  }
}

Also notice we can't set a string array to NBT.

@WesCook
Copy link
Owner

WesCook commented Jun 24, 2018

You know OreDicts are based on Item and metadata which means I have to maintain a magical list of ItemStacks for every single case

That makes sense. I was thinking about it from the opposite angle (attach an oredict to an ItemStack), when it's really "add itemstack:meta to big list". So I agree, the oredict approach is not viable.

And I just have a new idea: is it a chance to make each nutrient has different value?

It would be possible, but for the reasons explained about I'd rather not make that change. I think your first suggestion for NBT makes the most sense right now.


Okay, so let's say mods can now add native support to Nutrition. How might that play out? Some further considerations:

  1. Is it possible for modpack authors to customize these complex foods? In part using CraftTweaker. May depend on the mod.
  2. Can these tags be removed? Yes, again with CraftTweaker. No easy way to override mods using the jsons alone (without adding new features like a "removal list").
  3. Which should have higher priority, json or mod support? I'm not sure yet.
  4. Should mods only add support for complex foods, or all foods? Possibly I should start moving the onus of nutrient support onto other devs...
  5. Will this further cement the 5 food group system, discouraging custom health systems? I think so, unfortunately. I do like to see custom food groups but the majority of players seem to stick to the defaults.

The other thing to consider is if the plan is to move full support over to the other modder's side, then which system would be easier for them: API, or NBT? NBT means no soft dependencies, and easier to build their mods. But a full API could offer useful features related to the mod, such as reading a list of available nutrients.

Feeling a bit of choice paralysis on this one, so I'm glad to talk to another dev who this would affect.

@3TUSK
Copy link
Contributor

3TUSK commented Jun 24, 2018

Assuming NBT were the best approach, would you mind giving an example of how the NBT might look so that Nutrition could read it?

I am collaborator of @Snownee's project who wrote almost all of the "cook" system code. Let me try to explain the situation we have right now:

First of all, I have an ItemStack whose NBT data has this layout:

{
  "ingredients": [
    {
      "material": "meetball",
      "size": 3
    },
    {
      "material": "pasta",
      "size": 8
    }
  ],
  "seasonings": [
    {
      "spice": "tomato_sauce",
      "size": 3
    }
  ],
  "effects": []
}

(Note: that effects is an empty list. In fact, it may contain extra effect which will be applied onto player when player eats the dish.)

So yeah, I roughly implemented a culinary system which allows player to make a dish with arbitrary ingredients and seasonings. (Think "enchanted golden apple pie" or "pizza with chrous fruit and ender pearl topping".) And the question is that OreDict does not recognize NBT data, nor Capability data. Furthermore, you can notice that there is no data that explicitly states that "it's spaghetti", so even OreDict supports NBT, I cannot use OreDict to distinguish them.
Therefore we are facing the core issue: if this dish supports Nutrition, there must be some way to allow it to increase players' nutrition data on a per-dish basis. So...

  1. From what I can tell, currently Nutrition mod relies on a data-driven system that stores records of Item -> Nutrition Value, and look up it on-demand. The easiest solution I can think of is an interface CustomNutritionValueProvider which has one method List<Nutrient> getFoodNutrients(ItemStack eatingFood). It may be realized as a capability.
  2. Our system would not be modpack-friendly, as the final nutrition value of a dish is depending on what the dish has. It may be possible to allow modpack creator to modify the nutrition value on a per-ingredient basis though.
  3. For the priority, my personal opinion is to treat such CustomNutritionValueProvider (as mentioned in 1.) as an override of JSON-based records.
  4. I have a MaterialCategory enum which divides all ingredients into different groups. Although it roughly matches the 5 nutritions, I still list "supernatural" as an individual category, for things like chrous fruit and ender pearl. So... yeah, I don't think providing native supports to Nutrition mod would solidify the 5 nutrition group system.

Hope this helps.

@TheIllusiveC4
Copy link

I'm the developer of Culinary Construct, a mod about making custom sandwiches, so I'll weigh in here on some of my thoughts.

I'm not a huge fan of the NBT route because it sounds rather clunky, lets modders bypass some or all internal logic set up by Nutrition, adds some unnecessary processing overhead, and they're not config-friendly. As an example of the last point, say a sandwich has steak as an ingredient and we add an NBT tag that assigns "protein" as a nutrient to the sandwich. After that, if we decide to change steaks to provide "superspecial" (custom food group) as well, this change is not reflected in the sandwich because there is no response to re-compute the NBT tag. Also, in my case, I would still need a soft dependency on Nutrition to find any custom food groups that could be registered to add an NBT tag for it.

A simple API that offers a way of passing in an ItemStack and returns a List<Nutrient> as mentioned above by @3TUSK would be more suitable.

I have one new method to suggest that I would perhaps advocate for the most, and that is to incorporate IMC to process a Function<ItemStack, List<ItemStack>> message. Store these functions in a registry of some kind and then use it to translate an ItemStack into a List<ItemStack> when calculating its nutrition contents.

The benefits of this method are no soft dependencies on Nutrition required and you guarantee that every mod acts within the design doctrine of Nutrition because you still handle all of the internal calculations. The downside is that this is fairly inflexible, perhaps some mods don't necessarily have a list of ingredients per say but still require a way of specifying certain nutrients or doing their own calculations. But this downside is also just a hypothetical, it's hard to say how this could best be accounted for without a specific functionality in mind.

@3TUSK
Copy link
Contributor

3TUSK commented Jun 26, 2018

Uh, to be exact, it's not returning a List<Nutrient>, but ObjectToFloatMap<Nutrient> (primitive-typed Map<Nutrient, Float>). I just had a closer view to the mod and realized that it has a similar design to that of TerraFirmaCraft...

@TheIllusiveC4
Copy link

I'm assuming you're suggesting a Map<Nutrient, Float> in order to assign different values to each nutrient? If so, I'm against this for the same reasons that @WesCook mentioned above about introducing that level of granularity to the system. At most, perhaps there should be a method float getNutritionValue(ItemStack food) included in the interface to specify a nutrition value override.

@WesCook WesCook reopened this Jun 26, 2018
@WesCook
Copy link
Owner

WesCook commented Jun 26, 2018

Thanks for your inputs, everyone. I appreciate your thoughts on how an API might look. Would like to settle on a system that supports VanillaFoodPantry, Culinary Construct, and most others.

I did have a thought based on the NBT example above which I'd like to suggest. What if we allowed the configs to understand NBT structure, so that they could support a large number of complex food types without requiring an API?

Full nutrient example:

{
	"name": "vegetable",
	"icon": "minecraft:carrot",
	"color": "72dd5a",
	"food": {
		"oredict": [
			"listAllveggie",
			"listAllgreenveggie"
		],
		"nbt": [
			{
				"item": "customFoods:sandwich:5",
				"nutrients": {
					"ingredients.material": "lettuce",
					"seasonings.spice": "tomato_sauce"
				},
				"value": "serving.size.path.here"
			}
		],
		"items": [
			"minecraft:carrot"
		]
	}
}

The nutrients and value fields are keys that describe the NBT "paths" in the ItemStack. It's basically suggesting Nutrition to follow the NBT path, and look for the listed strings in those locations. If any are found, the nutrient is confirmed as being in that ItemStack.

value could support either an NBT path, or a hardcoded number. Though Java might give me some flack about allowing either type.

Pros:

  • Fully config-based. There's no shift in the relationship between Nutrition/other mods.
  • Most custom food mods are likely using NBT, so should be supported out of the box.
  • Custom food groups are just as viable as before.

Cons:

  • The structure above is still ambiguous as it's specifying the nutritional value in one nutrient's config. If another nutrient file specifies a different value, then we have a conflict. I could just throw an error in these cases, but that feels unintuitive.
  • Increased complexity of configs (albeit necessary).
  • I'm guessing a lot of mods generate the food value dynamically, instead of storing it in the NBT. This means hardcoding the nutritional value would be required and may not be very accurate.

Do you think this approach makes sense? It may be "too techy" for some modpack authors, but isn't much more complex than writing CraftTweaker scripts.

@TheIllusiveC4
Copy link

For my case at least, that approach would not be ideal.

If I understand this correctly, every possible variant of the NBT data would have to be accounted for to list them in the configs like that. Since any item that extends ItemFood is a possible data value for my custom sandwiches, this means listing out every applicable food item in the config under the NBT tag in addition to listing out that item normally for base Nutrition. I can imagine this same issue would apply to most sorts of composite foods that may have dozens to hundreds of possible values.

@3TUSK
Copy link
Contributor

3TUSK commented Jun 26, 2018

One additional con: that approach also creates dependency on implementation details - the data structure may change over time, even though it's unlikely.

@WesCook
Copy link
Owner

WesCook commented Jun 26, 2018

Good point, Illusive. This would not support your system of allowing all ItemFoods at all. While we could feed it back to the main item list, I feel that's probably too specific to your use case, and really not very intuitive behavior.

3Tusk, it's true that this behavior may change over time. Though that caveat exists for the current system as well (modid:item:meta), so it's not a huge departure.

Do you both feel an IMC-based approach is better than a config-based approach, then? Or is there some way to retool this so that it supports all use cases?

@TheIllusiveC4
Copy link

TheIllusiveC4 commented Jun 26, 2018

I personally can't see a way to make a config-based approach to this problem that doesn't border on being very hacky or counter-intuitive. Rather than try and directly change the config structure, I think it's best to prioritize config-compatible approaches if possible.

While I did advocate for an IMC-based approach before, I've had time to reconsider. It works very well for my use case, but I can see how it might be too limited in functionality for other use cases. At this juncture, I think some design decisions have to be made. Do you wish to prioritize config and modpack support or modder support? I know you mentioned this dilemma before, but this is really where the crux of it lies.

If you prioritize the config and modpack support, then I would say IMC would be best. The IMC method I outlined above (with a small tweak) would basically be designed to expand any given ItemStack into a List<ItemStack> of its base ingredients. These could be translated directly into the internal Nutrition dictionary defined by the config files, perfectly preserving the power of the config with basically zero notice on the consumer-end.

However, this means that all mods looking to implement support will need to follow this rigid structure and they may not be able to work around it. For instance, it would be impossible to properly implement if, for whatever reason, its ingredients are not represented as an Item at all or if the modder wishes to implement nutrition in a non-standard fashion.

If you prioritize modder support, then I would say that an interface-based/capability API approach would be best. Something that provides a method returning List<Nutrient> from an ItemStack would give modders the most flexibility in how they process nutrients, and I could imagine it covering most use cases. However, this would require a soft dependency on Nutrition, it completely bypasses the config files (unless the modder specifically supports it), and you give up some ability to control the nutrient calculation logic.

There's also an argument to be made for maybe implementing multiple approaches depending on the mod's specific needs, but that's ultimately up to you to decide if it's worth the effort.

@WesCook
Copy link
Owner

WesCook commented Aug 3, 2018

Thanks everyone for your input, and I'm sorry I've not responded to this until now.

I saw an example on reddit earlier and it made me reconsider this discussion. @V0idWa1k3r recently added Nutrition support to his new mod ExPetrum (which looks great by the way). There's no Nutrition API yet so he was forced to use reflection. I'd like to rectify that so there's at least some avenue for modders to add official support. V0id, I'd be happy to hear your input on this topic too.

So I'd like to work on an API. While there were pros to the other approaches discussed above (eg. NBT is simple and has no dependencies), I now think a proper API will be necessary. Information like "which nutrients are enabled", or "what is the player's current nutrition?" would be useful to know, and could be made available.

Knowing that, would you folks be willing to add a soft dependency to your mods for Nutrition? I don't want to get ahead of myself if not.

Assuming that's a good option though, I'd like to create a stable API which allowed both simple and complex foods as discussed above.

I've not created an API in Java/Forge before so you'll have to bear with me on this. Many of you are more experienced than me so I would appreciate any input on design choices.

On the issue of modder vs modpack author control, I still want modpack authors to have the final say. There will be a config to disable IMC access, and likely a method to granularly disable nutrients for certain foods. Understand this may be important for heavily-customized packs like SevTech.

In the longer term, I'd also like to slowly shift the responsibility of adding mod-support from my curated list of jsons to an API used by other mod authors. Now that the mod has some popularity I think it's a more reasonable ask. Those authors will also be better at keeping up when adding new foods, and may have different ideas about what nutrients their foods should really represent. (eg. KnightMiner considered slime to be dairy - who knew? 😛)

I was pretty worried about solidifying the five food-group system, but I think that ship may have sailed already. Tyranny of the defaults and all that. As long as modpack authors still have a way to safely change those nutrients, I'm not going to worry about it so much going forwards.

So, an API. That's going to require some things.

  • A proper capability interface. The current interface was really only made for internal use. It will need some additional functionality to support external use. Input welcome on that topic.
  • A release of the sources jar, and Maven repo to compile against.
  • A small sample mod to demonstrate the API.

That's as far as I've gotten. Please let me know if you have any thoughts/criticisms before I get started, as this is the best time to consider any changes.

Thanks!

@TheIllusiveC4
Copy link

Knowing that, would you folks be willing to add a soft dependency to your mods for Nutrition?

I'm fully willing to add a soft dependency on Nutrition for my mod.

@Wabbit0101
Copy link
Author

I am ok with a soft dependency for a Nutrition capability interface. Also +1 the idea regarding the mod authors keeping nutrition list up-to-date; it's much easier for me to keep that sync'd when I make a new release that it is for you to do so!

@3TUSK
Copy link
Contributor

3TUSK commented Aug 4, 2018

Nutrition compatibility is still on my TODO list - although it would be a bit tricky (I do have my own opinion regarding modder vs. modpack dev control), I will see what I can do to adapt.

@WesCook
Copy link
Owner

WesCook commented Aug 4, 2018

I do have my own opinion regarding modder vs. modpack dev control

I'm more than happy to hear it. :)

@KnightMiner
Copy link
Contributor

On the topic of the five nutrient system, have you considered having people register custom ingredient types other than item stacks? Basically, if I have a dynamic sandwich, instead of returning a list of veggie, grain, etc, I return a list of bread, tomato, etc, then in the config files I can specify how those custom ingredients relate to a nutrient type.

@WesCook
Copy link
Owner

WesCook commented Aug 4, 2018

So you're thinking a sort of keyword list, which mods could identify their foods as having? Then it the jsons:

{
	"name": "vegetable",
	"icon": "minecraft:carrot",
	"color": "72dd5a",
	"food": {
		"oredict": [
			"listAllveggie",
			"listAllgreenveggie"
		],
		"items": [
			"minecraft:carrot"
		],
		"ingredients": [
			"tomato",
			"lettuce"
		]

	}
}

These ingredients would then be attached as NBT data to the ItemStacks (or some other way). The idea being that modders wouldn't be committing to the five food groups specifically, and players/modpack authors could repurpose these for their own needs.

Is that right?

@WesCook
Copy link
Owner

WesCook commented Aug 4, 2018

So I see a few pros and cons here.

Pros:

  • It does help preserve the freedom of modpack authors which I'm certainly a fan of
  • Removing the focus on ItemStacks may also be less "techy" for modpack authors.
  • For complex foods (eg. custom sandwiches), adding these tags would be fairly intuitive.
  • There wouldn't be the requirement of reading in the current nutrient-list from an API first.

Cons:

  • By applying abstraction (eg. mod:babycarrot > carrot), we're potentially losing information.
  • Maintaining an ingredients list would likely increase upkeep costs of existing jsons.
  • It feels a bit like we're duplicating the oredict system, doesn't it?

(I do love my pros and cons lists)

Overall I think this system probably makes a lot of sense for complex foods, as first talked about in this issue. I'm not sure it's easier for simple foods though.

By adding a layer of abstraction we'd be loosening the contract between Nutrition and the end-mod. It sounds like this would be closer to the NBT approach discussed above.

@3TUSK
Copy link
Contributor

3TUSK commented Aug 4, 2018

@WesCook
Put in short, our mod is intended to mimic the culinary art in real life, and we expect that there are some mechanics that players' culinary skill will directly affect how many nutritions, or even what types of nutritions they will get after eating the food they cooked. For instances, if you overcooked a dish that contains vegetables, I don't think that player will still gain nutrition in vegetable category; at the same time, eating raw/not-very-cooked meat (of any kind) may still give you nutrition in meat category, but I figure that it will be inefficient, which will be reflected as decrease in nutrition gain.
As such, I do want to retain a certain level of control on mechanics of our mod; a list like what @KnightMiner proposed may fit well, but I am not certain about further customizability. I am not even sure whether the design of Nutrition conflicts with ours (and, if so, to what extents).

@WesCook
Copy link
Owner

WesCook commented Aug 4, 2018

@3TUSK I see. So it sounds that you'd like to be able to numerically specify each nutrient as well. So a burnt steak sandwich might give 0.5 protein and 1 grain, whereas a well-done steak sandwich may give more protein, right?

Unfortunately I'd still like to avoid getting that specific with nutrients. I understand your mod focuses more on realistic precision, whereas Nutrition sort of straddles the line between realism and not.

I think the best way to go there would be to offer thresholds. So if a burnt steak doesn't meet a certain threshold of protein, then the protein nutrient wouldn't be added to the resulting ItemStack.

I know that's likely not perfect, but may offer a decent compromise between our two systems.

@3TUSK
Copy link
Contributor

3TUSK commented Aug 6, 2018

I wonder if it is acceptable to manually increase player's nutrition on my side?

@WesCook
Copy link
Owner

WesCook commented Aug 6, 2018

I've been considering that same question. I'm currently thinking about implementing a two-tier system:

The first tier would use IMC exclusively. This would allow mods to broadcast a list of foods and their associated nutrients at game start. Essentially an extension of the json system. This removes the requirement of a soft dependency, and should be easy to implement for mods which add only a handful of simple foods.

The second tier would use capabilities, which do require a soft dependency. This would allow reading and writing information directly, and offer you the control needed for more complex uses. I'd need to whip the capability into shape so that it's more appropriate for external use.

So simple foods would ideally be sent as IMC, whereas more complex foods (eg. custom sandwiches, or culinary creations) would be handled on your side. There's a lot of details to work out still, but that's the tentative idea right now.

Does that seem a reasonable approach to all of you?

@3TUSK
Copy link
Contributor

3TUSK commented Aug 6, 2018

Yes, from what I can tell that would be sufficient for me to implement the "rather complex" system.

@WesCook WesCook changed the title API to tell Nutrition about adhoc composite foods Nutrition API Aug 8, 2018
@XvBUSHvX
Copy link

XvBUSHvX commented Aug 8, 2018

I have contacted the developer for "FoodFunk" with the following suggestions, and a request to assist in implementing the compatibility feature detailed below in #31

The Decay mechanic from FoodFunk adds a nice touch of immersion. I think it would be beneficial to expand upon that mechanic through compatibility with another mod, by the name "Nutrition", that provides nutritional values to food which can applies a custom buff/debuff. In most cases, "Nutrition" is able to take the food eaten and assign accumulated nutritional percentages.

It is my hopes that between both developers, a solution could be found that would allow a foods typical nutritional value(I believe "Nutrition" currently bases their stats on a food saturation) could be reduced by the percentage of decay from "FoodFunk". Then the difference could be applied to another nutritional category, if implemented by the user. Which would normally result in a negative debuff, from consuming too much decay.

@maxanier
Copy link

maxanier commented Aug 24, 2018

Not sure where to put this, but since it is API related, this might be the right spot.
I'm the developer of the mod Vampirism and I have been asked to implement a certain compatibility to this mod.
TeamLapen/VampirismIntegrations#20

Vampirism allows the player to become a vampire. Vampire players do not need eat but suck blood. Hence, they can/should not care about the nutrition values from this mod. Whereas players who decide to become a hunter instead can still think about their nutrition values.

If you were to create an API using the second approach (capability), could you include some way to manually set the nutrition values or somehow disable the system? There is a dedicated Vampirism Integrations mod, so I would not care about the soft dependency.

Edit
I could probably implement this with the current code, but I would prefer a "stable" API

@WesCook
Copy link
Owner

WesCook commented Aug 24, 2018

Hey @maxanier,

This is the best place for the request. So disabling Nutrition on a per-player basis -- I think that would be doable via the capability API.

I can't promise an ETA because modding is a secondary priority for me after real work, but I'll keep your request in mind as I develop the API. Thanks for your comment.

@3TUSK
Copy link
Contributor

3TUSK commented Sep 29, 2018

@WesCook Apologize for pinging - for better or worse, our mod is now visible-source now: https://github.com/Snownee/Cuisine. You can find our usage of Nutrition at here.

@WesCook
Copy link
Owner

WesCook commented Sep 29, 2018

Congrats on the release! I'm sorry there's not been much progress on my end, but I'm glad you've been able to add in your own support in the mean time.

@codewarrior0
Copy link

codewarrior0 commented Feb 11, 2019

Here's another jerry-rigged Nutrition compat module to throw on the pile: NutritionCompat.java. Lot of duplicated code here - wish I could just ask Nutrition for a tooltip, and to add player stats, given the food value and nutrient list.

@WesCook
Copy link
Owner

WesCook commented Feb 12, 2019

I know I've done a poor job in building this out. Having some real-world examples of how it might be used is actually very helpful though, so thank you for sharing.

The mod itself looks interesting, too!

@WesCook
Copy link
Owner

WesCook commented Feb 28, 2019

I just wanted to give an update on this. I've spent the last few days working on changing how data is handled by the mod to give more authority to the server. This would be a necessary prerequisite to letting the server handle API requests, before passing information on to the client.

Unfortunately I ran into some technical hurdles that I'm not sure how to overcome right now. I'm not a Java developer so it may be an issue of inexperience, but I believe more serious refactoring is required to proceed. Some early assumptions I made in designing the mod would no longer apply (particularly when dealing with sidedness), and that creates some difficult complexities.

As I don't have a clear direction right now, I sadly have to declare this API project on indefinite hiatus. I'm sorry for engaging you and then not delivering. I thank you for your ideas and thoughts all the same.

I am willing to work with another developer to find solutions if one is open to collaboration. Though of course I understand we're all pretty busy as is. :) Feel free to send me a message or reply below if you're interested.

Otherwise, thank you for reading and taking an interest in my silly little mod.

@3TUSK
Copy link
Contributor

3TUSK commented Feb 28, 2019

Although I may not have enough time, I would like to ask what are technical details about your issues?

@WesCook
Copy link
Owner

WesCook commented Mar 1, 2019

Sure, I'll try to give some insight.

The biggest assumption I made early on is that the NutrientList class would be loaded early and made available to both the client and server. This class stores the actual json data after it's been loaded in and cleaned up.

To support an API however, I'd want to grant the server full authority over loaded nutrients. The server can then handle any changes via API, and send it to clients as they connect. This is admittedly how I should have done it from the beginning (had I thought ahead).

I ran into a few problems when attempting to make this change.

As there's no logical separation between client and server yet, I moved the json loading functions from PostInit to FMLServerStartingEvent. This is late enough that there's a logical separation between client and server. However it causes problems for JEI which looks at tooltips for all items during early loading.

I realized that configs would also need to be sent over the network to be consistent, which means they're now being loaded after the client loads the world instead of preInit. Not to mention the Effects data as well (at least if I want to implement #146).

Not having a static class of NutrientList meant I had to store a separate instance for the client and server. The client's would be a local "dummy" object, and the server's would be the canonical version. Some cases are obvious which to use: any GUI function uses the client version, and any chat commands use the server version. However utility functions like NutrientUtils were less-obvious, and I had to start writing hacks to specify if it was a server or client variable in the method parameters. This felt like a major code smell but I wasn't sure of a better approach. Maybe there's some way to abstract the reference to always pull from the appropriate source?

It also seemed very redundant that the jsons would be parsed on the server, stored as Nutrient objects, and then deconstructed back down to simple types to serialize for network packets. Then they need to be deserialized on the client back into meaningful data.

I'd love if I could just send the json data directly, because then I could run the same clean up process on both client and server. The problem is that doesn't accommodate any changes made via API. Perhaps there's a clean way to serialize an arbitrary object, but I don't know of any.

Another assumption I made early on is that foods would be ItemFood. Of course things are never that simple. I've already extended definitions to BlockCake and ItemBucketMilk, but I really need to stop any type checking and simply see if an item or block can be "used".

I admit I was also getting pretty overwhelmed trying to design an API that would be powerful enough to suit all use cases, without being overly complex or unstable. I hit upon a two-stage API design that would understand IMC for simple stuff and capabilities for more advanced stuff. It sounded like a good approach, but I didn't get far enough to test it out.

I wrote some design details up. I still have the notes, but ultimately I had a lot of unanswered questions. Even basic questions like:

Should Nutrition store the canonical nutrient data (and keep all clients abreast), or should it just add hooks for other mods to intercept?

Unfortunately I don't have the experience with Java or API design to be very confident in solving those problems. Maven repos? API versioning? Totally new concepts to me.

I ended up putting things off because I wasn't sure what to do. When I finally put some time in to try and implement the prerequisites, I ran into the technical issues described above. That's when I decided to officially call it off instead of giving false hope any longer.

Hopefully that gives you a better idea of some of the problems I was having. I'm sure they're far from unsolvable; I just don't have the solutions myself. Not right now anyway.

@Wabbit0101
Copy link
Author

A little short for time but my immediate thoughts in response to your concerns:

  • As the author of the original posted issue, it's not a problem at all if you decide the issue is not doable at this time. Some we win; some we lose. Now as to you specific issues:
  • Regarding server(master), client(slave). This is a common enough scenario that perhaps you can just find another well supported mod that is doing the same thing and emulate. Maybe just put your ideas into a branch and let the modders on this thread have a look? TheOneProbe comes to mind as a visual mod that does everything serverside mostly; maybe you can reach out the McJty for tips.
  • Regarding what appears wasteful and redundant regarding server/client json and other syncing. I'd just not worry about this too much. EVERYTHING is Minecraft core and modded is being pushed this way for good and bad. (Working on flattening my mods from having a few dozen json files to having literally hundreds cured me of this worry...at first I tied myself up thinking 'there has GOT to be a better way'...nope there ain't.) I'd say focus on only having the simpliest data you need and worry about optimization iff it proves to be an issue once things are working.
  • Regarding being overwhelmed designing the API to end all APIs and cover all possible uses. Don't do that. Pick one or two uses cases to solve (eg plain old ItemFoods...use the 80/20 rule). Design and code only-as-much-as-you need for those two with the new server/client setup. Create dummy items in a test mod (or use food mods...I'll help). When you're satisfied with your new design, add another use case (perhaps one that is orthogonal to the first two -- for instance something that is edible but NOT identified as ItemFood like cake) and iterate. This way you're always working of something that is working. Don't be afraid to declare prerequisites for integration with your mod either. For instance, items that are not ItemFood must be declared as part of an item group 'consumables' or have an action of either EAT or DRINK or whatever to work with Nutrition and then test that. Check out other food APIs like IEdible from AppleCore. Maybe you add a 3rd use case to support IEdible derivatives and piggyback off the work another modder has already done; then focus on testing that. Arbitrary drinkable Fluids are also a PITA, so maybe create an integration rule or FML api or capability or whatever; declare your requirements, and test a simple use case. At some point you have to say 'good enough!' and publish the V1 integration guidelines you've come up with so far. Your own test mod will provide the starting point for other modders to update their own code.
  • And always: if you're feeling a bit overwhelmed or stumped or just annoyed: take a break; leave the code for awhile; let your unconscious do the problem solving.

Just my thoughts.
HTH, The_Wabbit (lemme know if you'd like the issue closed for now)

@WesCook
Copy link
Owner

WesCook commented Mar 2, 2019

Hey @Wabbit0101,

Thanks a lot. I appreciate your words, and your helpful suggestions.

I did stash my code changes so I could put those on another branch if that's of use. If I make another attempt I'll probably just rebase though. Sometimes doing things over can offer new ideas.

Yes, I think I'll mark this closed for now. That's not to discourage discussion though. I'm still very open to hearing new comments, suggestions, or even requests (should I make another attempt in the future).

Thanks again for your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request or expansion
Projects
None yet
Development

No branches or pull requests

9 participants