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

Microwave recipes now uses stacktype id instead of entity prototype id for stacked entities #28225

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

blueDev2
Copy link
Contributor

@blueDev2 blueDev2 commented May 23, 2024

About the PR

Microwave recipes now treat stacked entities by their stacktype id instead of entity prototype id.

Why / Balance

This fixes an issue with making medicated sutures and regenerative mesh using brutepacks and ointments from a medical techfab.
Medical techfabs make "Brutepack1" and "Ointment1" entities and stacking them makes them retain their entityid, but the recipe used "Brutepack" and "Ointment" entities, so it was impossible to use brute packs and ointments from techfabs to create advanced medicine.

Technical details

Microwave recipes now uses stacktype id instead of entity prototype id for stacked entities

Media

video1811032388.mp4
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Any microwave recipe that uses a stacked entity must now instead use the entity of the base stack entity, usually the base stack entity name does not have a number appended to the end of it.
Also when a stacked entity is used, each
Stackable entity prototypes can be found at Resources\Prototypes\Entities\Objects\Materials or whatever equivalent for the specific fork

As an example, here is the changes to the recipe for Regen Mesh

  • type: microwaveMealRecipe
    id: RecipeRegenerativeMesh
    name: regenerative mesh recipe
    result: RegenerativeMesh
    time: 10
    solids:
    FoodAloe: 1
    Ointment: 1 -> Ointment: 10
    MaterialCloth1: 1
    reagents:
    Sigynate: 20
    Dermaline: 20

Changelog

🆑 blueDev2

  • fix: Fixed microwave recipes that use stacked materials

Copy link
Member

@AJCM-git AJCM-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix linter, only had a glance at the code but it seems like it's a bit longer than necessary? try see if there's a way to shorten it or you are checking things unnecessarily (also use proxy methods like Del() instead of direct EntityManager calls)

@AJCM-git AJCM-git added the S: Awaiting Changes Status: Changes are required before another review can happen label May 31, 2024
@blueDev2
Copy link
Contributor Author

blueDev2 commented May 31, 2024

Just to make sure, you want me to change to linter so it doesn't fail when doing this right? Cause I checked the errors and while they are correct since stackid is not entityid, the point of this PR is to make the recipes entity-agonistic.

Copy link
Member

@AJCM-git AJCM-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, had a bit more time to think about this and i'm pretty sure what you actually need to do here is not touching the recipes at all, but instead when you check for stack component, if it has it then you index the prototype and do solidID = prototype.Spawn; and thats all you need (i think)

@blueDev2
Copy link
Contributor Author

I don't think I understand what you mean by "index the prototype". I cannot find any field called "Spawn" at the moment.

@blueDev2
Copy link
Contributor Author

Oh wait, the stack prototype?

@blueDev2
Copy link
Contributor Author

Ok I see what you mean now, but couldn't that easily introduce confusion since, for example, a recipe that requires "SheetGlass1: 10" and a recipe that requires "SheetGlass10: 10" would both just require 10 steel? Or would it be a rule to only use the SheetGlass1 variant?

@AJCM-git
Copy link
Member

AJCM-git commented May 31, 2024

Ok I see what you mean now, but couldn't that easily introduce confusion since, for example, a recipe that requires "SheetGlass1: 10" and a recipe that requires "SheetGlass10: 10" would both just require 10 steel? Or would it be a rule to only use the SheetGlass1 variant?

Yeah the amount in the recipe takes absolute priority so people should just use the base variant with just 1 in the stack, microwaves are absolutely atrocious code anyways so i would prioritize fixing the bug.

Yet another option that is cleaner but more annoying is making the base Brutepack and Ointment have 1 (thus replacing their variants with 1 and making the techfab print these) in the stack by default and add and Ointment30 prototypes as replacements for current references to brutepacks and ointments (its a bulk replace so it prob wouldnt be that bad)

@blueDev2
Copy link
Contributor Author

Ok I see what you mean now, but couldn't that easily introduce confusion since, for example, a recipe that requires "SheetGlass1: 10" and a recipe that requires "SheetGlass10: 10" would both just require 10 steel? Or would it be a rule to only use the SheetGlass1 variant?

Yeah the amount in the recipe takes absolute priority so people should just use the base variant with just 1 in the stack, microwaves are absolutely atrocious code anyways so i would prioritize fixing the bug.

Yet another option that is cleaner but more annoying is making the base Brutepack and Ointment have 1 (thus replacing their variants with 1 and making the techfab print these) in the stack by default and add and Ointment30 prototypes as replacements for current references to brutepacks and ointments (its a bulk replace so it prob wouldnt be that bad)

You meant Ointment10 right? They only stack to 10.

@blueDev2 blueDev2 requested a review from AJCM-git June 1, 2024 01:40
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Jun 1, 2024
@AJCM-git AJCM-git added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Jun 3, 2024
Comment on lines 204 to 241
{
var metaData = MetaData(item);
if (metaData.EntityPrototype == null)
string? itemID = null;

// If an entity has a stack component, use the stacktype instead of prototype id
if (TryComp<StackComponent>(item, out var stackComp))
{
itemID = _prototype.Index<StackPrototype>(stackComp.StackTypeId).Spawn;
}
else
{
var metaData = MetaData(item);
if (metaData.EntityPrototype == null)
{
continue;
}
itemID = metaData.EntityPrototype.ID;
}

if (itemID != recipeSolid.Key)
{
continue;
}

if (metaData.EntityPrototype.ID == recipeSolid.Key)
if (stackComp is not null)
{
if (stackComp.Count == 1)
{
_container.Remove(item, component.Storage);
}
_stack.Use(item, 1, stackComp);
break;
}
else
{
_container.Remove(item, component.Storage);
EntityManager.DeleteEntity(item);
Del(item);
break;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now, the last thing that irks me about this, couldnt you just merge this into a single if else and just repeat the itemID != recipeSolid.Key check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would work but I think the current structure makes more sense:
First we check the entity and get an ID
Then check if that id is the one we are currently looking for, if not, iterate to the next entity
Then remove the entity or decrement the stack component count

An outer loop is counting down the amount of solids needed to be removed, so every iteration of this loop will only remove 1 entity or decrement the stack by 1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh, its just a shorter version of basically the same thing but i wont block this over that

@AJCM-git AJCM-git removed the S: Awaiting Changes Status: Changes are required before another review can happen label Jun 4, 2024
@AJCM-git AJCM-git merged commit 0e93d13 into space-wizards:master Jun 4, 2024
12 checks passed
@blueDev2 blueDev2 deleted the Microwave-Stacks-Fix branch June 4, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants