Skip to content

Harpy rework#646

Open
Holorain4312 wants to merge 28 commits intoProjectOmu:masterfrom
Holorain4312:Harpy-rework-real
Open

Harpy rework#646
Holorain4312 wants to merge 28 commits intoProjectOmu:masterfrom
Holorain4312:Harpy-rework-real

Conversation

@Holorain4312
Copy link

@Holorain4312 Holorain4312 commented Mar 5, 2026

About the PR

Why / Balance

This PR makes it so that harpies are faster at using tools that their job requires them to use more often.
It lets medical harpies do surgery faster and and security harpies reload mags with bullet faster.

on the flipside it makes tools that they arent proficient in slower to use

the idea is that this isn't a flat out buff to harpies but a rework.

Technical details

Added a new prototype for proficiencies

Provided Access to ToolComponent and BallisticAmmoProviderComponent to the system made for this PR

Media

2026-03-06_08-09-26.mov

Requirements

Breaking changes

None to note

Changelog
🆑

  • tweak: Reworked harpies, they are now faster at using tools they are proficient in and slower at using other tools.

@Holorain4312
Copy link
Author

Holorain4312 commented Mar 5, 2026

damn changelog broke it seems

Edit: Fixed it! :D

@github-actions github-actions bot added the S: Untriaged Has not been set a status; currently not labeled. label Mar 5, 2026
Copy link
Member

@Thesia000 Thesia000 left a comment

Choose a reason for hiding this comment

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

SO if I am reading this code correct you are setting the proficency GLOBALLY I recomend moving the proficency into the Proicency component instead of having it as a prototype that is saved as a variable inside a system

@Holorain4312
Copy link
Author

SO if I am reading this code correct you are setting the proficency GLOBALLY I recomend moving the proficency into the Proicency component instead of having it as a prototype that is saved as a variable inside a system

So like just save the variable to the component and just reference the component instead?

@Holorain4312 Holorain4312 requested a review from Thesia000 March 6, 2026 01:31
@github-actions github-actions bot added the S: Needs Review Needs review by a maintainer. label Mar 6, 2026
@JustOrdinaryPao JustOrdinaryPao added A: Character/Species Area: Player characters and species features and content. Type: Balance Change Changes something in the game related to balancing. and removed S: Untriaged Has not been set a status; currently not labeled. labels Mar 6, 2026
Copy link
Member

@Thesia000 Thesia000 left a comment

Choose a reason for hiding this comment

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

for the changes you need to read and save to the entities component not into a variable in the system itself. it will work for local tests but fails once we hit multiplayer as multiple entities will have this comp and override the variable in Proficency System

@Holorain4312
Copy link
Author

for the changes you need to read and save to the entities component not into a variable in the system itself. it will work for local tests but fails once we hit multiplayer as multiple entities will have this comp and override the variable in Proficency System

OHHHHH shi mb gang

@Holorain4312 Holorain4312 requested a review from Thesia000 March 6, 2026 20:37
@JustOrdinaryPao JustOrdinaryPao added Type: Enhancement New feature or request, change to game. S: Needs Second Approval Approved by another maintainer, just needs one more to check it. and removed S: Needs Review Needs review by a maintainer. labels Mar 6, 2026
Copy link
Contributor

@NotActuallyMarty NotActuallyMarty left a comment

Choose a reason for hiding this comment

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

ive not looked at the balance i can chud out later at that but you should omumod this instead of putting this in Content.Shared._Omu

@Holorain4312
Copy link
Author

ive not looked at the balance i can chud out later at that but you should omumod this instead of putting this in Content.Shared._Omu

good point for omumodding it, i'll do that when i'm free

balance has been approved by vivi
image

@Holorain4312
Copy link
Author

okay so @NotActuallyMarty it seems it actually HAS to be in the _Omu folder because otherwise the references to it ToolComponent and BallisticAmmoProviderComponent will not be able to access it

@3PonPon3
Copy link
Contributor

Does this mean non-med and non-sec harpies strictly get debuffs for it

@Holorain4312
Copy link
Author

Does this mean non-med and non-sec harpies strictly get debuffs for it

For reloading/surgery? Dont think i actually gave specific debuffs for those outside the fields, probably should have and can easily do at any point

@3PonPon3
Copy link
Contributor

Oh it ONLY affects surgert and reloading? For sll harpies I mean

I thought it would affect things ljkr prying, syringes, using tools, etc etc

@Holorain4312
Copy link
Author

Oh it ONLY affects surgert and reloading? For sll harpies I mean

I thought it would affect things ljkr prying, syringes, using tools, etc etc

So for tools, the way i have it setup:
If the job isnt specified in the yaml, this rework doesnt affect it.
If there is a modifier there, and the tool selected isnt one you are proficient in, you get a negative speed modifier to it; this REALLY comes into effect with med having debuffs to all traditional tools but buffs to surgery.
Also sci will get debuffs for jaws of life experimental welders and drills

@3PonPon3
Copy link
Contributor

So what does it affect?
I see tools, reloading, surgery tools

@Holorain4312
Copy link
Author

So what does it affect?
I see tools, reloading, surgery tools

Yeah thats it for now

@3PonPon3
Copy link
Contributor

So harpies in the specified jobs when non sec reload slower? >.>

@Holorain4312
Copy link
Author

So harpies in the specified jobs when non sec reload slower? >.>

I didnt actually specify debuffs for reload speed because it uses a slightly different system there afaik, basically i had to use a different value for reload speed and just didnt get around to giving literally every non combat role specified a debuff.

If there was a job i though specifically shouldnt be able to reload fast id have specified a negative modifier there tho

@3PonPon3
Copy link
Contributor

Eo for example hop is completely unnafected but for another example secharpy reload fast n what use base tools faster but surgery slower?

@Holorain4312
Copy link
Author

Holorain4312 commented Mar 11, 2026

Eo for example hop is completely unnafected but for another example secharpy reload fast n what use base tools faster but surgery slower?

Probably shouldve given engi a negative surgery modifier LMAO
Engi gets base tools and upgrades faster, sec harpy reloads faster, sci harpy gets base tools and a small surgery buff cause robotics exists

HoP is completely unaffected though, yes

TLDR when i get around to it im giving engi a surgery and reload debuff because their buffs are kinda extremely powerful without

@EctoplasmIsGood
Copy link

“In today’s unique and cool PR I decided to buff harpies and made up a reason to do so”

@Holorain4312
Copy link
Author

“In today’s unique and cool PR I decided to buff harpies and made up a reason to do so”

Dude this is a bounty for vivi, i probably shoulve put that in the title but it is

@rat-dot-net
Copy link

“In today’s unique and cool PR I decided to buff harpies and made up a reason to do so”

🥜🥜🥜

Comment on lines +33 to +36
## Highly specialised

Harpies are extremely good at doing tasks they are familiar with. they can use most tools their job specialises in more efficiently than everyone else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you specify here for the players that don't check github which tools they specialise in?

Copy link
Author

Choose a reason for hiding this comment

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

yeah, can do

little busy till like sunday tho so it'll take a lil bit

Copy link
Contributor

@NotActuallyMarty NotActuallyMarty left a comment

Choose a reason for hiding this comment

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

Did some code cleanup, I recommend giving it a once-over and make sure i didn't fuck something up. There were just too many nested statements making code hard to read. Kept functionality the same afaik, so give me a sec to re-review that.

Comment on lines +1 to +44
- type: Proficiency
id: BaseTools
items:
- Crowbar
- Wrench
- Screwdriver
- Wirecutter
- Welder
- Multitool

- type: Proficiency
id: advancedTools
items:
- WelderExperimental
- PowerDrill
- JawsOfLife

- type: Proficiency
id: BaseSci
items:
- Crowbar
- Wrench
- Screwdriver
- Wirecutter
- Welder
- Multitool
proficiencyMultiplier: 1.2
# give sci a minor surgery speed buff, robotics exists and needs surgery to function
surgeryProficiency: 1.1

- type: Proficiency
id: BaseEngi
items:
- Crowbar
- Wrench
- Screwdriver
- Wirecutter
- Welder
- WelderIndustrial
- Multitool
- WelderExperimental
- PowerDrill
- JawsOfLife
proficiencyMultiplier: 1.2
Copy link
Contributor

Choose a reason for hiding this comment

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

This is bad. You might want to consider some type of parenting system, or listing proficiencies based on some other factor, i.e. checking for a ToolQualityPrototype of a specific kind or such. That way you plan for future additions and don't have to manually add every tool to a list if somebody adds a new tool.

For example, abductor tools here are not supported.

Copy link
Author

Choose a reason for hiding this comment

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

i was planning on doing this, i couldn't find a way where i could make it work at that time, but i probably just forgot something; i'll have a look into it later

Comment on lines +1 to +15
- type: Proficiency
id: CargoTechnician
items:
- Crowbar
- Wrench
- Screwdriver
- Wirecutter
- Welder
- Multitool
proficiencyMultiplier: 1.1

- type: Proficiency
id: ShaftMiner
items:
- Crowbar
Copy link
Contributor

Choose a reason for hiding this comment

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

See other Comment about changing the base proficiencies, that way you could add a singular thing here (i.e. - BasicTools for example) and wouldn't need to list out every item individually.

Ideally you should make the JobId of the respective job a datafield on the prototype rather than making the prototype name match the JobId.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, you could add more jobs to this datafield, that way you don't need multiple prototypes to set the same proficiency to multiple jobs.

Co-authored-by: NotActuallyMarty <27968892+NotActuallyMarty@users.noreply.github.com>
Copy link
Contributor

@NotActuallyMarty NotActuallyMarty left a comment

Choose a reason for hiding this comment

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

Math is bad and Prototype refactor.


if (TryComp<BallisticAmmoProviderComponent>(args.Unequipped, out var ammoComp))
ammoComp.FillDelay *= MathF.Pow(entity.Comp.ReloadSpeedProficiency, -1);

Copy link
Contributor

Choose a reason for hiding this comment

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

You should not just assume that setting SpeedModifier of the Item to SpeedModifier^(-1) thus setting it to 1 is the default, as tools can have negative or positive speed modifiers, and this would just set them to 1 permanently.

Check the prototype of the item, get its speedmod, and set it to that on unequip.

Comment on lines +79 to +85
if (args.JobId == null)
return;

entity.Comp.ProficiencyID = args.JobId;

if (!_prototypeManager.TryIndex<ProficiencyPrototype>(entity.Comp.ProficiencyID, out var proficiencyPrototype))
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

See other comment, You should support multiple JobIds per prototype so you don't need to make redundant prototypes for duplicate modifiers.

Copy link
Contributor

@NotActuallyMarty NotActuallyMarty left a comment

Choose a reason for hiding this comment

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

fixing

Co-authored-by: NotActuallyMarty <27968892+NotActuallyMarty@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: Character/Species Area: Player characters and species features and content. S: Needs Review Needs review by a maintainer. S: Needs Second Approval Approved by another maintainer, just needs one more to check it. size/M Type: Balance Change Changes something in the game related to balancing. Type: Enhancement New feature or request, change to game.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants