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

Secondary experience #38013

Closed
roaringjohn opened this issue Feb 14, 2020 · 9 comments
Closed

Secondary experience #38013

roaringjohn opened this issue Feb 14, 2020 · 9 comments
Labels
Crafting / Construction / Recipes Includes: Uncrafting / Disassembling <Enhancement / Feature> New features, or enhancements on existing

Comments

@roaringjohn
Copy link
Contributor

Is your feature request related to a problem? Please describe.

It seems that secondary skill requirements don't get experience in recipes.

Describe the solution you'd like

Allow secondary skill requirements to gain (significantly?) reduced experience from recipes, probably depending on the skill. For example, one should not get exp for archery if it's a secondary skill requirement, but one should get exp for cooking.

Describe alternatives you've considered

Allow multiple main skill requirements?

@ifreund ifreund added (S2 - Confirmed) Bug that's been confirmed to exist <Bug> This needs to be fixed <Enhancement / Feature> New features, or enhancements on existing Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Feb 14, 2020
@ifreund
Copy link
Contributor

ifreund commented Feb 14, 2020

What should happen imo is that skill gain is made proportionate to the impact of the skill on failure chance. This would mean that same amount of total xp is gained, but distributed among the skills. With the current numbers this would mean 75% of xp goes to the primary, and the remaining 25% is distributed evenly between the secondary skills.

Changes should be isolated to this function:

void player::craft_skill_gain( const item &craft, const int &multiplier )

For reference, the failure chance calculation is done here:

double player::crafting_success_roll( const recipe &making ) const

@ifreund ifreund added the Good First Issue This is a good first issue for a new contributor label Feb 14, 2020
@roaringjohn
Copy link
Contributor Author

Keep in mind that one should probably want to put in more work than what ifreund suggested, as it makes no sense for combat skills to get xp from crafting. For this, ifreund said on Discord:

yeah you could special case it, i think we already have some conception of skill categories
if that route is taken it might make sense to apply the same special casing to failure chance as well

@kevingranade
Copy link
Member

As you note with archery skill when making as arrows, some secondary skills should not be gaining xp.

The intent is that secondaries act as prerequisites, but are not exercised as part of the craft. If you want to change this to mean, "skills also exercised by the craft", we need to overhaul a lot of recipes. Step one would be to blanket remove all combat skills from crafting recipes, which I would fully support. After that you need to audit the remaining secondaries for whether each recipe provides meaningful practice for that skill, or if it just acts as a prerequisite. If both of those are occurring in meaningful numbers, we can split secondary into two parameters, with the new one being "prerequisite_skill".

@scorpion451
Copy link

I could see justification for combat skills getting some experience from lower level recipes, representing learning how to properly maintain and handle the equipment. (Learning how to properly string and store a bow, how to align a gunsight, getting a feel for weapon balance as you craft them, that sort of thing)

@GGgatherer
Copy link
Contributor

I could see justification for combat skills getting some experience from lower level recipes, representing learning how to properly maintain and handle the equipment. (Learning how to properly string and store a bow, how to align a gunsight, getting a feel for weapon balance as you craft them, that sort of thing)

To me, what you're describing looks more like "allow any recipe to train combat skills but up to a set skill level". Like 1 or 2, maybe.

@KorGgenT
Copy link
Member

we'd definitely need some kind of flag to stop this, as all of my recipes in magiclysm that have spellcraft as a secondary skill are purposefully like that so spellcraft doesn't gain exp.

@ifreund ifreund removed the Good First Issue This is a good first issue for a new contributor label Feb 15, 2020
@TechyBen
Copy link
Contributor

I could see justification for combat skills getting some experience from lower level recipes, representing learning how to properly maintain and handle the equipment. (Learning how to properly string and store a bow, how to align a gunsight, getting a feel for weapon balance as you craft them, that sort of thing)

And balance. Forging a sword would give a lot of knowledge to balance, gait and pose for the sword. Not for much fighting, but for the beginning levels of knowledge and skill.

Who's more dangerous, an experienced builder with a sledge hammer, or a novice swordsman on the first day?

@kevingranade kevingranade removed the <Bug> This needs to be fixed label Mar 7, 2020
@Night-Pryanik Night-Pryanik removed the (S2 - Confirmed) Bug that's been confirmed to exist label Aug 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@github-actions github-actions bot added the stale Closed for lack of activity, but still valid. label Dec 5, 2022
@kevingranade
Copy link
Member

This was more of a discussion than an issue, and it has gone stale with no real conclusion.

@stale stale bot removed the stale Closed for lack of activity, but still valid. label Dec 5, 2022
@kevingranade kevingranade closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crafting / Construction / Recipes Includes: Uncrafting / Disassembling <Enhancement / Feature> New features, or enhancements on existing
Projects
None yet
Development

No branches or pull requests

8 participants