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

Chemprentice #2646

Closed
wants to merge 22 commits into from
Closed

Chemprentice #2646

wants to merge 22 commits into from

Conversation

Temoffy
Copy link
Contributor

@Temoffy Temoffy commented Jan 2, 2025

About the PR

New smaller less powerful chem master intended for cooking and botany QoL! Currently strips out the pill/bottle tab and limits it to 500 units.

also implemented the unused loaded chemmaster sprite

Why / Balance

ChemMasters are very nice to have for cooking and botany, but full chemmasters are hard to access short of highgrading for... reasons.

It's time for cooking to advance beyond juggistry and enter a new era of easier to handle chems.

How to test

place ChemPrentice from entities admin menu, build chemprentice with ship, buy from flatpack vender.

Media

unused ChemMaster sprite implemented:
image

ChemPrentice in action:

chemprenticev20001-2890.mp4

Requirements

Breaking changes

Changelog

🆑

  • add: Added new ChemPrentice machine to flatpack vend!

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 2, 2025

Must-dos before merge: strip bottle and pill related things from the code and from the yml, get at least slightly different sprite.

Want-to-dos before merge: allow internal reactions, limit buffer to 400-1k units, make table placeable and lighter to drag, add chip, add flatpack to vend, add t1-t2 tech.

@dvir001
Copy link
Contributor

dvir001 commented Jan 2, 2025

All added code and files have to sit under frontier namespace

_NF

@dvir001
Copy link
Contributor

dvir001 commented Jan 2, 2025

This look like insane amount of copy paste and while its not a bad thing to have it self-contained into its own corner, this can use a lot of existing code for everything except the UI it seems.

Why not try to reuse then copy when possible?

Also your machine can use the chem master as a parent then a full copy paste of the yml.

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 2, 2025

Most of the classes copied from are sealed or I would be pointing at existing code more often besides ReagentButton which I will be testing if I can use the extising one. And good catch with the yml parenting, I thought I would have to remove components instead of just modifying the itemslots component.

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 3, 2025

Think that's the last I need to touch the code unless I missed some more cleanup somewhere.

Gonna make a basic sprite and circuit board for it.

Should I add flatpack/vending/research/lathe/construction in this PR or make that later?

Copy link
Contributor

github-actions bot commented Jan 4, 2025

RSI Diff Bot; head commit 9400677 merging into cdcc0dd
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/_NF/Structures/Machines/chemprentice.rsi

State Old New Status
prentice_empty Added
prentice_full Added
prentice_unlit Added

Edit: diff updated after 9400677

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 4, 2025

PR edited with new sprite and stuff.
Should I just roll with a CC_BY_4.0 or is it fine as is? Validator is fussy but I think it's up to the boss folks.

@Temoffy Temoffy marked this pull request as ready for review January 4, 2025 17:08
@dvir001
Copy link
Contributor

dvir001 commented Jan 4, 2025

You have to use the license formats
image

Also im not sure what you mean by adding this.

Format need to be CC-BY-4.0

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 4, 2025

I mean I wouldn't want to see my work in an ERP server is all, but I'll swap it

@MagnusCrowe
Copy link
Contributor

Very cool little tabletop device. Looks like a futuristic coffee maker!

Does it spill its buffer when it's destroyed or blown up?

@Havaren
Copy link
Contributor

Havaren commented Jan 9, 2025

Can we request the Pill function be left? its how we make food pills and candies.

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 9, 2025

If the pill making part is kept, it'd just be a limited buffer chemmaster, which some are already pushing for. I don't think that would be accepted.

@dustylens
Copy link
Contributor

I concur with Temoffy. If candy is desired that can be satisfied elsewhere.

This tool is ideal for more common access for people who have use for the mechanical features of the chemmaster but without the industrial scale and full range of features it provides.

@gitjubx
Copy link

gitjubx commented Jan 10, 2025

500u is a very very small...storage ..aswell as no pillmaking or bottle making...it will only have a nish usage . only good usage will be for filterting chemicals a bit from plants or cooking leftover materials (which if there potent plants people wont bother the hassle of jugging and filtering all the time how frequent you need to do it if a plant has 30-40 chems per ( would make it atleast 1k-2k units and maybe ability to be more with upgrades 1k+(t2) 1k+ (t3) +2k (t4 bluespace) ) but the pill feature needs some polishing overall like ability to select which chem you wanna put into a pill (chemmaster needs that) so you dont have to decide if you have one for making pills or one for storing stuff ( but that maybe belongs to another topic) . From my personal experience with chemmasters and kitchen/botany with the low amount of storage it has aswell it mixing chemical inside? if i readed correctly its worse than a keg (besides that you can filter but have the mixing conflicts aswell the time it takes to filter chemicals into seperate container ) . It is a good idea however to give kitchen finally its own sort of chemmaster but it will just have a very nish usage with the current stats and if there is the possiblity to pay 10k for a chemmaster most people always will opt for that

@gitjubx
Copy link

gitjubx commented Jan 10, 2025

Must-dos before merge: strip bottle and pill related things from the code and from the yml, get at least slightly different sprite.

Want-to-dos before merge: allow internal reactions, limit buffer to 400-1k units, make table placeable and lighter to drag, add chip, add flatpack to vend, add t1-t2 tech.

allow internal reactions means it will for example if there is water and flour in the chemmaster buffer make dough on its own? (no cryobeaker effect in buffer) because that would remove one of the mayor purposes be able to print cheese/cake pattern/dough if you have the chemical stored in chemmaster

@dustylens
Copy link
Contributor

Oh yeah allowing internal reactions would probably be... Bad.

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 10, 2025

The reaction part was removed at dusty's request, the original post has up to date info on it.

if there is the possiblity to pay 10k for a chemmaster most people always will opt for that

Good, I don't want a machine available at roundstart to be as useful as what the developers decided was a luxury machine only available from expeds/vroids.

As Dusty said, this isn't industrial scale. It's meant to help clean up juggistry cooking results and sort out slightly mutated plants. I expect it'll find use in sauce and drink mixing, but it's role is primarily cleaning and sorting not production.

That said, I probably should make the buffer size upgradable.

@gitjubx
Copy link

gitjubx commented Jan 10, 2025

there always be ways to get one on roundstart ,and if you cant get one then the other machines comes to the chemmaster, just saying to make it decent so it wont cause that much a gap and making it most people just aim for the alternative ...upgradeable storage size is a good start , maybe the feature of making candy instead of pills would lessen the effect of the low storage more ( since you cant make pills but you dont wanna have the same as chemmaster) then there sure would also be people that just buy it for that single reason...beeing able to make candy ...but prop they still would aim for the alternative but less likly to not buy one of these too then

Copy link
Contributor

@MilonPL MilonPL left a comment

Choose a reason for hiding this comment

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

sort of rushed, i'll look over it again at some point

Comment on lines +78 to +147
/// <summary>
/// Update the container, buffer, and packaging panels.
/// </summary>
/// <param name="state">State data for the dispenser.</param>
private void UpdatePanelInfo(ChemPrenticeBoundUserInterfaceState state)
{
BufferTransferButton.Pressed = state.Mode == ChemMasterMode.Transfer;
BufferDiscardButton.Pressed = state.Mode == ChemMasterMode.Discard;

BuildContainerUI(InputContainerInfo, state.InputContainerInfo, true);

BufferInfo.Children.Clear();

if (!state.BufferReagents.Any())
{
BufferInfo.Children.Add(new Label { Text = Loc.GetString("chem-master-window-buffer-empty-text") });

return;
}

var bufferHBox = new BoxContainer
{
Orientation = LayoutOrientation.Horizontal
};
BufferInfo.AddChild(bufferHBox);

var bufferLabel = new Label { Text = $"{Loc.GetString("chem-master-window-buffer-label")} " };
bufferHBox.AddChild(bufferLabel);
var bufferVol = new Label
{
Text = $"{state.BufferCurrentVolume}/{state.BufferMaxVolume}",
StyleClasses = { StyleNano.StyleClassLabelSecondaryColor }
};
bufferHBox.AddChild(bufferVol);

foreach (var (reagent, quantity) in state.BufferReagents.OrderBy(x => x.Reagent.Prototype)) // Frontier: add OrderBy
{
// Try to get the prototype for the given reagent. This gives us its name.
_prototypeManager.TryIndex(reagent.Prototype, out ReagentPrototype? proto);
var name = proto?.LocalizedName ?? Loc.GetString("chem-master-window-unknown-reagent-text");

if (proto != null)
{
BufferInfo.Children.Add(new BoxContainer
{
Orientation = LayoutOrientation.Horizontal,
Children =
{
new Label {Text = $"{name}: "},
new Label
{
Text = $"{quantity}u",
StyleClasses = {StyleNano.StyleClassLabelSecondaryColor}
},

// Padding
new Control {HorizontalExpand = true},

MakeReagentButton("1", ChemMasterReagentAmount.U1, reagent, true, StyleBase.ButtonOpenRight),
MakeReagentButton("5", ChemMasterReagentAmount.U5, reagent, true, StyleBase.ButtonOpenBoth),
MakeReagentButton("10", ChemMasterReagentAmount.U10, reagent, true, StyleBase.ButtonOpenBoth),
MakeReagentButton("25", ChemMasterReagentAmount.U25, reagent, true, StyleBase.ButtonOpenBoth),
MakeReagentButton("50", ChemMasterReagentAmount.U50, reagent, true, StyleBase.ButtonOpenBoth),
MakeReagentButton("100", ChemMasterReagentAmount.U100, reagent, true, StyleBase.ButtonOpenBoth),
MakeReagentButton(Loc.GetString("chem-master-window-buffer-all-amount"), ChemMasterReagentAmount.All, reagent, true, StyleBase.ButtonOpenLeft),
}
});
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like at least some of this could be defined in xaml instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it can it's beyond my knowledge, and any changes here should probably be mirrored to the chemmaster xaml

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 11, 2025

there always be ways to get one on roundstart ,and if you cant get one then the other machines comes to the chemmaster, just saying to make it decent so it wont cause that much a gap and making it most people just aim for the alternative ...upgradeable storage size is a good start , maybe the feature of making candy instead of pills would lessen the effect of the low storage more ( since you cant make pills but you dont wanna have the same as chemmaster) then there sure would also be people that just buy it for that single reason...beeing able to make candy ...but prop they still would aim for the alternative but less likly to not buy one of these too then

I thought candy was just sugar and flavor in pills? are they something else? And if its a bit under 'decent' on first implementation, that's better than being overpowered. If it needs tweaking it can be tweaked, but it kinda needs testing first. We'll see how and how often it gets used.

@dvir001 dvir001 added S: Awaiting Changes This PR has changes that need to be made before merging and removed S: Awaiting Changes This PR has changes that need to be made before merging labels Jan 12, 2025
@dvir001 dvir001 requested a review from MilonPL January 12, 2025 13:20
@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 12, 2025
Copy link
Contributor

@MilonPL MilonPL left a comment

Choose a reason for hiding this comment

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

no reason to mirror the changes to the ChemMaster/upstream stuff because that will just cause conflicts in the future
a full refactor should be done upstream, but old code doesn't need to follow conventions, while any new Frontier code should

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 12, 2025

done

@whatston3
Copy link
Contributor

Not a huge fan of this.

If more chemical separation is required, why not take the existing service extractor, make a bigger version of it, make it researchable, give it to scientists. Job done, you can separate your enzyme from your corn oil or remove the unwanted bits from your bartending oopsies. You give more demand for stuff and avoid introducing a new machine with a new UI that needs to be maintained.

An infinite source of flatpack ersatz chemmasters available at round start trivializes access to chemistry equipment, removes demand for any interaction with specialized chemistry ships.

If you want a ChemMaster for pill production or mass chemistry separation, great, purchase it.

The beaker in the sprite is a nice touch. Thank you.

@blackknight954
Copy link
Contributor

Not a huge fan of this.

If more chemical separation is required, why not take the existing service extractor, make a bigger version of it, make it researchable, give it to scientists. Job done, you can separate your enzyme from your corn oil or remove the unwanted bits from your bartending oopsies. You give more demand for stuff and avoid introducing a new machine with a new UI that needs to be maintained.

An infinite source of flatpack ersatz chemmasters available at round start trivializes access to chemistry equipment, removes demand for any interaction with specialized chemistry ships.

If you want a ChemMaster for pill production or mass chemistry separation, great, purchase it.

The beaker in the sprite is a nice touch. Thank you.

the selective service dropper? I think that's already a research item

@dustylens
Copy link
Contributor

dustylens commented Jan 17, 2025

This is a tricky one for me.

I recognize the maintainer concern regarding if the juice is worth the squeeze in looking after this device. That's beyond my experience and I wouldn't want to speak for anyone else who would be charged with ensuring this device remains in working order.

I do feel that there is a place for a device that can separate out your reagents. Filter and separating is generally something a lot of service fields have call for. If the selective service dropper mechanics could be extrapolated on in a fashion that wasn't an enormous pain in the butt that could be pretty interesting.

Encouraging players to procure chem masters on their own is difficult to really embrace. At the moment, unless I am wildly out of the loop, those tools exist as loot and not a science path.

@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 17, 2025

What Dusty said, and I was under the impression the defining part of a chem ship was the dispenser, vend, and chem-making machines like the centrifuge and electrolizer. Not the chemmaster. I've seen chemmasters used a few times to great effect in cooking ships and I've had several people enthusiastic about a kitchen chemmaster, but I've never seen a cooking ship make meds or chems with one or heard people consider it.

Haven't personally tried flying a chem ship though so maybe sorting people's chems on request is a larger part of the job than I thought?

If it makes you feel better, the code and UI are just stripped back chemmaster with like ~15 lines of changes mostly pertaining to buffer limit and window size.

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the S: Merge Conflict This PR has conflicts that prevent merging label Jan 18, 2025
@Temoffy
Copy link
Contributor Author

Temoffy commented Jan 19, 2025

I get the impression this would never have been accepted, and now chemmasters are t2 researchable.

@Temoffy Temoffy closed this Jan 19, 2025
@Bonaout
Copy link
Contributor

Bonaout commented Jan 19, 2025

I feel like the Chemprentice still has a place on Frontier, as it really looks like it belongs in a kitchen and is a more accessible version of the chemmaster (being able to buy a flatpack instead of having to seek out a science ship).

It would be pretty sad if it didn't get added at all. It looks really cute and useful for chefs who want to be able to separate some ingredients without having to get a big chemmaster that feels out of place in a kitchen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# S: Merge Conflict This PR has conflicts that prevent merging S: Needs Review This PR is awaiting reviews Sprites UI YML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants