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

Proficiencies, crafting, failure chance, UI, etc #48981

Closed
Kelenius opened this issue May 21, 2021 · 62 comments
Closed

Proficiencies, crafting, failure chance, UI, etc #48981

Kelenius opened this issue May 21, 2021 · 62 comments
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Info / User Interface Game - player communication, menus, etc. Mechanics: Effects / Skills / Stats Effects / Skills / Stats (P2 - High) High priority (for ex. important bugfixes)

Comments

@Kelenius
Copy link
Contributor

Kelenius commented May 21, 2021

Describe the bug

I would like to collect all issues and RPs that are currently opened (there are too many, so my solution for solving that is to open one more), because there is a significant overlap between them and they boil down to the same problem:

The effect of having missing proficiencies on crafting and crafting failures in general are broken in various ways.

Existing discussions

Issues:
#46662
#47566
#48465
#48597
#48925
#48956

PRs:
#48658
#46082
#46153
#48673
#48650
#48924

And a discussion:
#48912

Why I'm opening this

Because all of this is very closely related and it's essentially the same problem that should be resolved in one go, not broken up into a bunch of PRs - merging #48673 fixed one problem, but also made crafting even very simple items practically impossible without proficiencies and left missing proficiencies impossible to mitigate because that's also broken - a worse situation, in my opinion!

I'd like to bring all discussion into the same spot, because right now it's spread out and is difficult to follow.

Let's untangle the knot

Here's the full list of what's currently bugged:

  • Any books that mitigate proficiency penalties are only applied to the UI. This is happening because for some reason - something about performance? - there are entirely separate functions for calculating time and failure chances of a recipe for the UI and for the actual crafting: UI's are in recipe.cpp, proficiency_time_maluses and proficiency_failure_maluses, while crafting's failures are handled in crafting.cpp, crafting_success_roll, and time in recipe.cpp, time_to_craft_moves.
  • Books that mitigate proficiency penalties also don't do it correctly. They simply multiply failure/time by their mitigation modifier, which leads to mag_animecon Boston AnimeCon magazine having 0.1, and you have prof_leatherworking_basic's time multiplier of 1.5 being multiplied by 0.1, resulting in 0.15 crafting time multiplier overall - or about 6 times faster. HOWEVER! Due to the bug above, this is only applied to the UI. In reality, the books have no effects on crafting time or failure chances. Relevant functions are, again, recipe.cpp, proficiency_time_maluses and proficiency_failure_maluses.
  • To make this even more confusing, there is a third place where those penalties are calculated: recipe.cpp, missing_proficiencies_string, which shows that "Proficiencies Missing:" line. It also simply multiplies time/failure chance by book's factor, but it also caps it at a minimum of 1.
  • But there's more! That "Time to complete:" line? Uses, ultimately, recipe.cpp, time_to_craft_moves, which means it's accurate to the actual crafting time, and isn't affected by the books.
  • Failure rate is handled very poorly. Relevant code is in activity_actor.cpp, craft_activity_actor::do_turn, near the end, going to crafting.cpp, item::handle_craft_failure, and then Character::crafting_success_roll. Here's the thing: your failures are checked at specific checkpoints; the next checkpoint is set when you start crafting and whenever you trigger one, and it's placed between your current progress and 100%, depending on the failure rate; being at 50% progress and rolling 0.1 would place the next check at 55%. Essentially, failure rate is double-dipping: it's both making the checks more frequent and making the actual failure chance higher. For example, having failure rate at static 0.5 (not actually possible, but bear with me) would put a checkpoint at 50%, then 75%, then 87.5%, and so on, and then on each of those you'd have a 50% chance of destroying every item in the craft - and every time you will also lose between 0.5 * 25 and 0.5 * 35 percent of the progress. And every time you are set back, the checkpoints are moved again - if you fail at 50% and get set back by 10%, you're now at 40% and your next checkpoint is at 70%. This creates a huge problem as your approach 100%, because the checks become more and more frequent, and you get set back by the same flat amount, which makes it insanely hard to reach 100% even with fairly low success rate. And to make it worse...
  • The impact the missing proficiencies have is absolutely insane. The actual check is, of course, not a flat 0.5. It's an opposed roll between your skill (number dice equal to weighted average between the primary and secondary skill, with 16+int dice sides) and craft difficulty (number dice equal to weighted average between the primary and secondary skill, with 24 dice sides); the skill roll is divided by the difficulty roll, and the result is your success rate. It is also divided by the effect of any missing proficiencies, which tends to absolutely destroy any success chance, especially when combined with the previous problem. For example, Principles of Leatherworking has a 5x failure rate.
  • A relevant problem is that a recipe that uses many different components is more susceptible to failures - every time a failure is checked, it rolls against every component.

How to resolve this mess

First we need to stop and asks ourselves: why am I spending my Friday evening digging through C++ code how difficult should it be for different characters to craft? There are three factors at play:

  • Character's intelligence vs the normal average of 8.
  • Character's skills (primary and secondaries, if any) vs craft difficulty
  • Any missing proficiencies that affect failure rate and whether or no they are mitigated; and if they are, then to which degree

(there are also NPCs and other small factors like crafting in darkness, but let's put that aside)

I see a lot of people arguing about how math works. That is not the approach that we should take. It would be, in my opinion, best to answer these:

"A character with 8 intelligence and the skill matching craft's difficulty should succeed XX% of the time."
"If that character is missing a 2x failure chance, they should succeed YY% of the time, which is halved success rate / doubled failure rate / something else, according to a formula."
"If that character instead has Z or more intelligence, they should always succeed."
"If that character's skill is instead W or more % higher than craft's difficulty, they should always succeed."
"If that character has 4 intelligence, their success rate should be (I am running out of good letters to use)%."
"If that character has skill (whatever)% lower than craft's difficulty, their success rate should be (something)%."

And then adjust the results to fit these. Failure checks should be more evenly spread across the recipe, and failures themselves should be more manageable. Right now they send you into a death spiral where you can't finish the craft because you keep getting set back by failures.

Second, and this is important, there needs to be one unified function for getting the penalties. Right now it's in four different places: recipe.cpp, proficiency_time_maluses and proficiency_failure_maluses; crafting.cpp, crafting_success_roll; recipe.cpp, time_to_craft_moves; recipe.cpp, missing_proficiencies_string.

Third, there needs to be a clear UI that shows what your actual failure chance is and how the proficiencies missing are affecting it. "Missing this proficiency makes you twice as likely to fail" isn't useful information when you don't know the base failure rate and how that 2x is applied to it. Showing how much proficiency progression you'd get from crafting the item is also something to consider.

@actual-nh
Copy link
Contributor

actual-nh commented May 21, 2021

Fourth, the code needs to be better-documented so it's clear to see exactly what is happening... EDIT: The below and related arguments are an indicator how much this is needed!

@actual-nh actual-nh added 0.F Feature Freeze <Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Mechanics: Effects / Skills / Stats Effects / Skills / Stats Info / User Interface Game - player communication, menus, etc. labels May 21, 2021
@deadcutlass2
Copy link
Contributor

deadcutlass2 commented May 21, 2021

  • your failures are checked at specific checkpoints. These checkpoints occur more frequently when your failure rate is high...

They are not. The output of Character::crafting_success_roll sets one singular failure point for the craft based on it's output. This is done in item::set_next_failure_point, line 1054 of crafting.cpp.

Attempting the same craft from scratch and attempting it at 99% have the same odds of succeeding. The difference is, assuming you roll a 0.6 from Character::crafting_success_roll, you'd end up at 60% completion and 99.6% completion respectively before failing, a 0.2 would give 20% and 99.2% respectively.

#48925 and #48956 should be resolved by #48924
#46662 and #48465 should be resolved by #48650 and #48658
#48597 should be resolved by #46082

#46153 should be the long term resolution to all of this.

@Kelenius
Copy link
Contributor Author

They are not. The output of Character::crafting_success_roll sets one singular failure point for the craft based on it's output. This is done in item::set_next_failure_point, line 1054 of crafting.cpp.

It's called multiple times, setting multiple failure points.

@pehamm
Copy link
Contributor

pehamm commented May 21, 2021

That is semantics, only one failure point exists at any one time, but the higher your expected failure, the higher the expected total number of failures until completion.

@Kelenius
Copy link
Contributor Author

That is semantics, only one failure point exists at any one time, but the higher your expected failure, the higher the expected total number of failures until completion.

I don't think you understand. Every time you trigger a failure, the next failure point is reset. In your example, if you are starting with 0% and 0.2 success rate, your next failure is 20% away, and if you start with 99%, it's 0.2% away. This creates a "clustering" effect where failures become more frequent as your approach 100%. And since every failure sets back your progress, that makes it very difficult to finish the craft.

@deadcutlass2
Copy link
Contributor

your phrasing indicated that the failure points are pre-chosen before even attempting the recipe, and that they were pre-chosen at the high end of a recipe. They are generated on the fly was my point.

@Kelenius
Copy link
Contributor Author

I suppose that's worth clarifying, but doesn't change my overall point.

@deadcutlass2
Copy link
Contributor

Are we waiting on I-am-Erk to check through the PRs or is Kevin willing to do them?

@ghost
Copy link

ghost commented May 21, 2021

I'm concerned that the "0.F Feature Freeze" tag means the soon(tm) stable will include the current 'impossible' crafting.

@actual-nh
Copy link
Contributor

I'm concerned that the "0.F Feature Freeze" tag means the soon(tm) stable will include the current 'impossible' crafting.

I can remove it if you'd like; it was more to make sure this doesn't go stale, and that I suggest that further fixing should definitely be a 0.G blocker.

@ghost
Copy link

ghost commented May 22, 2021

That is because the probability is tied to the item itself with the progress, if you have a 20% chance to pass each check you are expected to have an average pass rate of 20 out of 100 checks it presents to you. If it detects that you are passing too many checks and succeeding too often in the beginning, it starts to make you fail much more to compensate. The probability is reflected in the frequency of events to preserve the meaning of "20% chance to pass" you can treat this as a pool.

If you succeed too much you have depleted your average amount of success pool and need to start filling a failure quota, consecutive successes lower your amount of success chance by nature (or rather, have a lower frequency).

The more you succeed, the lower your effective success for passing threshold (to preserve what the pass rate means), because you have completed your success quota and it's time to fail, the more you fail the more the success chance can be "regenerated" and then have a chance to succeed.

With a high pass rate, your success will be lowered the more times you succeed, especially consecutively, and it will generate the failure threshold points depending on the expected result of failing the average amount and reflect it in a frequency of an event, this leads to the so-called clustering of failures, if you depleted your pool of expected successes, you will start to be forced to fail to fulfill the expected amount of failures the end, this regenerates the pool for success, then you can potentially finish the item (or just drawing out the failures that should have to happen on average).

The pool is tied to the item craft itself trying to preserve the frequency of events. At some point, you have to start "failure crafting" because it's necessary for the item to accept the completion of item progress.

Succeeding too much early can be setting yourself up for failures later.

Alternatively, failing early is a form of "failstacking" for success later, adding a bonus to effective success (compared to the last check) and are a necessary component to work towards item completion.

In the end, it should not matter on average, because they are just both quotas in a way to work towards item completion, if you can calculate the average amount of failures and successes necessary after incurring your penalties, you can find the quota.

@pehamm
Copy link
Contributor

pehamm commented May 22, 2021

That is because the probability is tied to the item itself with the progress, if you have a 20% chance to pass each check you are expected to have an average pass rate of 20 out of 100 checks it presents to you. If it detects that you are passing too many checks and succeeding too often in the beginning, it starts to make you fail much more to compensate. The probability is reflected in the frequency of events to preserve the meaning of "20% chance to pass" you can treat this as a pool.

This is not how any of this works. If you fail at 50%, your future progress does not depend in any way on whether you failed 10 times or 0 times, your probability of more failures going forward is exactly the same. Look up Gambler's Fallacy

@pehamm
Copy link
Contributor

pehamm commented May 24, 2021

I made some plots to illustrate how the proficiency factor changes the success probability. This is a recipe with 5 difficulty and attempted with 5 skill, 8 intelligence. No proficiency missing. To succeed the result of skill_roll has to be larger than diff_roll, or skill / diff_roll > 1. The left plot shows the density function of both rolls, which lie on top of each other because we fulfill the requirements exactly. The second plot shows the density function of the ratio skill_roll / diff_roll, with the probability to succeed being the orange area under the curve, which starts at x > 1.0.
Prof1

Now, if a recipe has a 2x failure multiplier, the ratio is divided by 2, which is equivalent to diff_roll being multiplied by 2. This leads to the following densities. The orange area is barely visible on the right plot (the area is equivalent to roughly 0.01%):
Prof2

This illustrates why the effect of proficiencies seems to be so extreme all of a sudden. I do not think that when the default failure multipliers were set to the levels they are, it was foreseen that the effect would be this large.

The plots were created by replicating the success_roll code in python, then creating 1,000,000 example rolls for each roll type and applying a Gaussian kernel density estimator.

@KurzedMetal
Copy link
Contributor

KurzedMetal commented May 24, 2021

merging #48673 fixed one problem, but also made crafting even very simple items practically impossible without proficiencies and left missing proficiencies impossible to mitigate because that's also broken - a worse situation, in my opinion!

Probably merging ANYTHING that improves this situation is better than we currently have right now.

All GitHub Actions buiilds from "Experimental Build #1" (about ~28 days ago) until now can't be played due to this game-breaking crafting issue. Specially because it's blocking key progress craftings like makeshift welders and reinforced solar panels.

@Kelenius
Copy link
Contributor Author

@pehamm Thank you! This is what I was trying to explain "The impact the missing proficiencies have is absolutely insane", but this is much clearer.

@KurzedMetal Merging the PR is what started all these problems - previously missing proficiencies increased your success chances instead of decreasing it, e.g. missing principles of leatherworking made your crafting 5 times MORE likely to succeed instead of LESS. When the PR got merged and the bug got fixed - that's where crafting became so difficult because all other bugs came to light - before that they weren't noticeable because the success rate became stupidly high which hid all problems.

@KurzedMetal
Copy link
Contributor

KurzedMetal commented May 24, 2021

I know that, I'm just saying the current state of the code & builds have been like this for 30 days and merging absolutely anything that alleviate the problem is better than leaving current state even more time. We already have some PRs trying to solve this but they are not getting reviewed or merged.

Also, keep in mind that the Launcher has started to deliver the latest GitHub Actions builds recently, so more players are being impacted by this issue and starting to create new threads in Reddit about it out of confusion (thinking that this is how the dev team wants proficiencies to work).

EDIT: Another example would be just reverting the PR that "fixed" proficiencies so it goes back to super high crafting rates until a more a holistic fix is prepared instead of having crafting not work at all.

@actual-nh
Copy link
Contributor

Ping: @kevingranade?

@ennob
Copy link
Contributor

ennob commented May 25, 2021

I feel like this shouldn't be postponed because of feature freeze. This is a game-breaking bug that effectively means it is impossible to craft certain items (makeshift welder and electrohack are recent examples where I got stuck). Can we add this to the blockers/To Do list for 0.F? Alternatively we could drastically reduce the failure chance (or the amount of progress lost) as a temporary fix until a rework can be completed.

@ghost
Copy link

ghost commented May 25, 2021

The difficulty rolls are normalized to the point on item progression (on the item counter itself), if you cleared up to that point you can still do more work towards item completion, you would just incur a penalty to your success and it will generate more failures along the way as you continue crafting.

This is how crafting any item works, people just interpret it all in a binary way when the difficulty rolls are tied towards the crafted item's degree of progress (item counter).

The difficulty of any crafting item even at full requirements gets increasingly harder to generate success as you progress towards item completion. But there is the illusion where it's out of player expectation that the difficulty is constant and they should be able to complete the item.

Since there's just an increased amount of ticks and multipliers that shifts your success rates and ability to make progress, failure point thresholds are just generated at levels you can not clear because the fail counter is incrementing up with every tick of failures that are being generated. At the same time, triggering them resets the counter and scales up your success chance by a small amount because it could be considered as part of a quota to fill to finish the item itself if you do not have the ability to clear the next segments of progress.

You just have to accept that you can't clear it in a single craft since you're required to do more work to complete the item if it's missing modifiers, but depending on how many you're missing it just becomes impossible to craft.

I just think that the numbers are a bit too higher, and it is out of expectation for people. Entry-level recipes should be scaled down to more reasonable amounts like ".5x more likely to incur failure" but convert that to "50% more likely to incur failure" so they know what to expect.

@ennob
Copy link
Contributor

ennob commented May 25, 2021

It is not just about expectation. For example in the latest case with the electrohack I consistently get to about 77%, fail, loose some materials, and get knocked back to between 50 and 60%. When I continue I get to around 77% again, only for the same thing to repeat. The problem is that I am stuck between 50 and 77%. I literally tried 15 times, spending more resources each time. If only I would have gotten slightly further each time it would have been OK, but I didn't.

If the player is not actually able to ever complete the item, then the item should have a higher skill requirement. Saying that "this is how crafting any item works" is not helpful.

One option to help fix this might be the following: After the player fails and looses some progress (and maybe some materials), make the player immune to failure until they reach the completion level where they previously failed. That way the player could still fail many times but he would at least make forward progress each time he invests more materials. It also makes sense from a story point of view since if the player just tried and failed some part of the crafting process, they are unlikely to make the exact same mistake again.

@Kelenius
Copy link
Contributor Author

Plus, the current crafting system makes it literally impossible to complete the item until you roll more than 1 success rate, because there will ALWAYS be a failure point between the current progress and 100%.

@ghost
Copy link

ghost commented May 27, 2021

The failure counter never resets unless it is triggered or until the item is complete and by nature is a precondition for completion, you have to scale things off the counter as well, afterwards you'll end up with huge numbers of expected failures between each failure point thresholds that can continually scale upward with a higher degree of progression. I kind of have to emphasize this again, failure is the only option to complete the craft.

Technically, you still doing the craft, but at a much higher difficulty, to some degree, I have to call it failure crafting, because it's part of the quota system being applied to the item counter. The degree of progress and difficulty scaling is tied to the exact tick on the progress counter, each should be calculated from the last, and the item counter will influence the difficulty settings to skew it towards a certain direction.

The difficulty settings might be tweaked too high too because it looks now the relationship between everything is reversed in some respects between base addition and multiplication for how the bonuses are applied now. If you ever stop crafting, you have triggered the counter effect, every "you lose progress" tick you're missing during a craft just adds to the counter by +1. When it's filled the failure condition triggers scaled to the difficulty of the craft and amount of times you trigger it. The problem is that it's triggered way too often for players to adjust to I feel, with the item counter it's actually enforcing a higher quota you have to fill.

In my definition, it's a "negative success roll" that you guys are trying to do for success roll. If you have to repeat the last segment, it's due to the item counter's effect pushing you down by skewing the results.

It looks like the proficiency multiplier that supposes to be a multiplication is now a division from the last change that influenced the order of operations on the math. I honestly don't think you can even do it through the success roll method because it enforces failure by definition, your simulation has to fluctuate and lose progress to be an accurate depiction with a chance of item destruction now, It's just part of actually crafting the item now.

@pehamm
Copy link
Contributor

pehamm commented May 27, 2021

1. Dramatically reduce fail multipliers in proficiencies. I suggest changing them to 1+ (1/10 current value), so a fail mult of 5 would become 1.5. This may be a bit too far, it will require a little testing... at a guess it's goign to lie somewhere between that and 1+(1/5 current) where the highest mult would now translate to 2.

Looking good. I simulated crafting an anvil because it is imo the most annoying recipe as of now due to the recipe needing both Principles of Metalworking and Blacksmithing proficiencies to craft and both proficiencies being annoying to practice without one (I am drowning in steel frames as one of the few viable options for blacksmithing).

Anyway, the recipe needs 3 fabrication, principles of metalworking has a 5 x fail_mulitplier, and blackmithing a 2.5 x fail_multiplier. With the old system this would have been a 5*2.5 = 12.5 divisor on the success roll, now it would only be 2.25. With average intelligence and 3 fabrication, we get a success probability of rought 0.05% and the following densities (again, probability densities of both rolls left, density of the ratio right, probability to succeed equal to the orange area under the density starting at x >= 1):
3 Skill

with 6 skill, we make massive progress getting 28% successes:
6 Skill

and with 9 skill, this grows to 95%:
9 skill

For comparison, with the old system I did not roll a single success in a million tries even with 9 fabrication.
9 skill old

@I-am-Erk
Copy link
Member

thank you very much for that @pehamm, that's exactly the sort of data I was looking for and a good choice of recipe. that suggests that if anything the 1+1/10 model might still be too tough, but it is definitely on a reasonable scale now.

This is an easy fix. I will set up a PR.

@ghost

This comment has been minimized.

@Zireael07
Copy link
Contributor

Whatever ends up done, the sheer amount of discussions and misinformation about this problem show that this part of the code needs to be well documented/commented to avoid getting broken sometime in the future again...

@ghost

This comment has been minimized.

@I-am-Erk
Copy link
Member

I-am-Erk commented May 28, 2021

Evrze, I don't mean to be harsh here, but I'd really appreciate if you'd back away from this topic. As best I can tell you don't have a really good grasp on it and your enormous posts have actively slowed me down from fixing it dramatically as I try to figure out what you're saying and how it relates to anything. I know you mean well, but it's clear you don't have a deep understanding of the issue; this is getting in the way.

@ghost

This comment has been minimized.

@I-am-Erk
Copy link
Member

Thanks for understanding. I still value your feedback, I just need a little less discussion at the moment.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@CleverRaven CleverRaven locked as spam and limited conversation to collaborators May 30, 2021
@Mom-Bun
Copy link
Contributor

Mom-Bun commented May 30, 2021

Locked for now as this issue seems to be getting spammed by one user in particular.

@CleverRaven CleverRaven unlocked this conversation May 30, 2021
@Raikiri
Copy link

Raikiri commented Jun 2, 2021

The experimental has lived for almost half a year with failure chances doing practically nothing and it worked just alright. Isnt it safer to release 0.F with failure chances just completely disabled until the issue is addressed properly?

The reasoning is that the current proficiency system still makes crafting quite hard (as in, way harder than it used to be) even from time penalties alone.

@I-am-Erk
Copy link
Member

I-am-Erk commented Jun 2, 2021

There's no need to disable them, they're working pretty well at this point. Only the book fix needs to be merged now.

@Raikiri
Copy link

Raikiri commented Jun 3, 2021

I understand that it's probably fixed now, but if during a feature freeze time, certain bug fixes can have dramatic impact on the gameplay (such as when it turns out certain stats had no effect in practice), then fixing them would produce more work (because now somebody needs to actually make those stats work). It's just natural to leave those types of can-of-worm bugs until after a milestone is reached.

Imagine if today somebody discovered: hey, turns out, vitamins have log messages about impacting your health, but their actual effects are disabled! Let's fix it now and re-enable them. While it's obviously unfinished content, in reality there's a good reason why it's disabled for now.

But it's just me hoping the next stable will be reached more smoothly.

@I-am-Erk
Copy link
Member

I-am-Erk commented Jun 3, 2021

It was a bit of a debatable choice to fix the failure rate bug in the first place rather than just disabling it, but despite the kerfuffle over it, it was not in fact a big deal at all to smooth it back out. Once it was done it made more sense to finish it off than to backtrack.

@I-am-Erk
Copy link
Member

I-am-Erk commented Jun 4, 2021

I believe this is ready to close, any objectors?

@Kelenius
Copy link
Contributor Author

Kelenius commented Jun 4, 2021

I agree, the big issues have been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Info / User Interface Game - player communication, menus, etc. Mechanics: Effects / Skills / Stats Effects / Skills / Stats (P2 - High) High priority (for ex. important bugfixes)
Projects
None yet
Development

No branches or pull requests