vgstation-coders / vgstation13

Butts
GNU General Public License v3.0
260 stars 542 forks source link

Standardises item blending #36617

Closed SECBATON-GRIFFON closed 3 days ago

SECBATON-GRIFFON commented 1 month ago

[bugfix][consistency]

What this does

takes the blend_items lists of mortars and grinders and combines their values onto vars on types of /obj/item, any values that are higher in the grinder list were kept over the mortar's. does the same with juice_items. cuts down code to make it reused by both items. makes grinding nettles actually give their chems right for both. Closes #34237. Closes #22024.

Why it's good

consistency

How it was tested

blend_juice unit test was written for most of this, grinder and mortar in-game otherwise

Changelog

:cl:

Eneocho commented 1 month ago

Novaflowers are supposed to give novaflour when ground, so you can bake novabread. Would this break that interaction and squeeze the capsaicin inside instead?

SECBATON-GRIFFON commented 1 month ago

Novaflowers are supposed to give novaflour when ground, so you can bake novabread. Would this break that interaction and squeeze the capsaicin inside instead?

thanks for catching that, reverting it to old value

Eneocho commented 1 month ago

Actually I'd like it if you could fix how the grinders handle the novaflowers, cause they always spit out 15u novaflour regardless of potency. Also, wheat grinds into flour instead of nutriment, does this handle that correctly? Unlike novaflour, the wheat's potency does increase the flour output.

SECBATON-GRIFFON commented 1 month ago

Actually I'd like it if you could fix how the grinders handle the novaflowers, cause they always spit out 15u novaflour regardless of potency.

consider it done

Also, wheat grinds into flour instead of nutriment, does this handle that correctly? Unlike novaflour, the wheat's potency does increase the flour output.

was always the case in the code

jwhitak commented 1 month ago

Exxion brought up a good point. Why have this long global list of types and specialty helper procs to determine if something can be ground up from this list? You could include these variables directly on the relevant objects, or have a proc that you can call to return them.

You even have the precursor of this:

/obj/item/proc/get_juice_amount()
    return 5

With specific subtypes returning different amounts. Now just replace that with an associative list of reagents and how much is contained, with the defaults being just nutriment, and overrides for individual items affected. The upside is that this can allow a juice/grind action to return multiple chemicals.

That said, I can see you not doing any of this because it's pretty beyond the scope of your already powerful bugfix. In fact, Exxion gave me a whole wishlist for a juice/grind rework, including merging the two functions, including the juice in the plants directly, and the blind/grind effect transferring the reagents and possibly additional ones. This is just a standarization PR lol. I'll leave the idea out in the open.

Eneocho commented 1 month ago

my two bits on the "including the juice on the plant already" idea: if you do make it such, make it so the juice depends on the potency and morphology and isn't on the reagents list itself, but rather as an extractable when crushed (so it behaves more or less like it does currently), cause you can splice shit really hard and the reagent cap in fruit is 100, so having a third or more of that cap consumed by the tomato juice is kinda lame. on the other hand we do have bananas, with them having the juice internally already, and you could just purge out the juice while splicing so it just doesn't take space, but that would clash with green grapes and carrots, since those have tannic acid (burn meds) and zeaxanthine (eye meds) respectively.

SECBATON-GRIFFON commented 1 month ago

Exxion brought up a good point. Why have this long global list of types and specialty helper procs to determine if something can be ground up from this list? You could include these variables directly on the relevant objects, or have a proc that you can call to return them.

You even have the precursor of this:

/obj/item/proc/get_juice_amount()
  return 5

With specific subtypes returning different amounts. Now just replace that with an associative list of reagents and how much is contained, with the defaults being just nutriment, and overrides for individual items affected. The upside is that this can allow a juice/grind action to return multiple chemicals.

That said, I can see you not doing any of this because it's pretty beyond the scope of your already powerful bugfix. In fact, Exxion gave me a whole wishlist for a juice/grind rework, including merging the two functions, including the juice in the plants directly, and the blind/grind effect transferring the reagents and possibly additional ones. This is just a standarization PR lol. I'll leave the idea out in the open.

well, i went ahead and did it anyways, but only made it support a single chemical and amount for now

jwhitak commented 3 days ago

Now the hardest part of the PR... updating the wiki 😈