Increase effect of prying quality on prying difficulty, JSONize prying_results #837

chaosvolt · 2021-09-02T22:42:19Z

Summary

SUMMARY: Balance "Unhardcode and JSONize prying results for terrain and furniture, increase effect of prying quality"

Purpose of change

Per discussion in #835, it was suggested that the effect of prying quality have more of an impact, noting that there was very little advantage to using a crowbar over a claw bar, and that this was an obstacle to balancing makeshift crowbars so that they can be allowed to open doors again.

Initial efforts to simply hack the desired features into the old hardcoded function led to being nudged to try and unhardcode it. After much fumbling about, with immense thanks to both Coolthulhu and OvenBaker, it's finally implemented.

Describe the solution

Converted prying info into a struct for terrain and furniture to make use of, which is then utilized by the crowbar function in iuse.cpp. This should be able to replicate basically everything the old prying function was built to do, from sounding the alarm to potentially breaking your windows on a failed pry.
The bonus for having a higher prying quality than needed is now defininable in the ter/furn object's JSON, which can be used to increase the impact that having a better tool has on prying a given. This is implemented to some extent, just enough to replicate the balance I was going for in the my initial commit (see below, includes links to effective chance of prying).
Time taken to use a crowbar was found to very very short even under non-ideal conditions, leading to a rebalance of how move cost scales with difficulty and strength.

Tables summarizing net impact of this will be below.

Describe alternatives you've considered

Leaving out usage of pry_bonus_mult for a separate PR, as already doing with the "doors can break on failed pry" mechanic.
It might be feasible to balance targets with minimum difficulty of 1 to work with 2x quality bonus instead of retaining the 1x multiplier, but it would require increasing the base difficulty for those to a degree as well. Seems like it'd be harder to balance out to work well since there's more room to expand.
Windows having the same difficulty as doors seems odd, could be nudged down a point.
Likewise, sealed coffins seem like they should be prying 1 tier, since if I recall the coffin construction implies you're nailing them shut, not locking them.

Testing

Compiled and load-tested.
Spawned in a claw bar.
Tested prying locked doors and windows, confirmed it pries stuff open and can fail.
Observed that prying windows can break on a failure, that it correctly changes terrain and spawns items.
Escalated to crowbar and continued waving prying tools at locked objects to test that game doesn't explode.

Additional context

Separate from but related to PR linked to above, making the balance impact of un-nerfing makeshift crowbars have less of an effect on the desirability of normal crowbars.

This should quantify the actual odds of a successful pry, before and after the changes, in a selection of contexts: https://anydice.com/program/2418a

Against locked doors specifically:

Context	Success Chance Before	Success Chance After
Prying 2, Strength 8	72.62% (Difficulty: 6)	46.98% (Difficulty: 8)
Prying 3, Strength 8	84.29% (Difficulty: 5)	84.29% (Difficulty: 5)
Prying 4, Strength 8	92.80% (Difficulty: 4)	99.51% (Difficulty: 2)
Prying 2, Strength 20	99.08% (Difficulty: 6)	97.17% (Difficulty: 8)
Prying 3, Strength 20	99.55% (Difficulty: 5)	99.55% (Difficulty: 5)
Prying 4, Strength 20	99.81% (Difficulty: 4)	99.99% (Difficulty: 2)

Time taken per pry attempt:

Context	Time Taken Before	Time Taken After
Prying 2, Strength 8	0.8 sec (Difficulty: 6)	3.2 sec (Difficulty: 8)
Prying 3, Strength 8	0.6 sec (Difficulty: 5)	2.2 sec (Difficulty: 5)
Prying 4, Strength 8	0.4 sec (Difficulty: 4)	1.2 sec (Difficulty: 2)
Prying 2, Strength 20	0.2 sec (Difficulty: 6)	2 sec (Difficulty: 8)
Prying 3, Strength 20	0.2 sec (Difficulty: 5)	1 sec (Difficulty: 5)
Prying 4, Strength 20	0.2 sec (Difficulty: 4)	0.2 sec (Difficulty: 2)

Implementing the ability to screw up and break doors on a failed pry will be added in a follow-up PR, per request.

Coolthulhu · 2021-09-03T07:50:14Z

I'd like to see some stats so that we aren't working on purely abstract proxies.
Something like https://anydice.com/program/24167 but with examples.

chaosvolt · 2021-09-03T14:56:53Z

When I get the chance later today I'll write up a table for it. It's definitely a bit of a weird roll:
if( dice( 4, difficulty ) < dice( 4, p->str_cur ) )

What about the humble claw bar?

That's one drawback I mentioned when discussing the different possible implementations to this, one side effect is it also nerfs the claw bar. It does still have the advantage of being much more portable, but I'm not sure if that's enough. Making it prying 3 would fix that but then make it a strait upgrade to the crowbar.

Conversely, the competing proposal was to allow prying 1 to work on everything, balance the scaling of prying bonus so that it's prying 1 that gets a difficulty increase and prying 2 that's nudged down to normal, have the makeshift crowbar retain prying 1, then sanity-check which other items have prying 1.

This would have the disadvantage of potentially nerfing other items that also have prying 1, and lead to trying to decide whether the standard hammer is beefy enough to warrant keeping that quality.

EDIT: Huh, where'd the comment I replied to go? o.O

chaosvolt · 2021-09-03T23:06:01Z

So, this should quantify the actual odds of a successful pry, before and after the changes, in a selection of contexts: https://anydice.com/program/2418a

Against locked doors specifically:

Context	Success Chance Before	Success Chance After
Prying 2, Strength 8	72.62% (Difficulty: 6)	46.98% (Difficulty: 8)
Prying 3, Strength 8	84.29% (Difficulty: 5)	84.29% (Difficulty: 5)
Prying 4, Strength 8	92.80% (Difficulty: 4)	99.51% (Difficulty: 2)
Prying 2, Strength 20	99.08% (Difficulty: 6)	97.17% (Difficulty: 8)
Prying 3, Strength 20	99.55% (Difficulty: 5)	99.55% (Difficulty: 5)
Prying 4, Strength 20	99.81% (Difficulty: 4)	99.99% (Difficulty: 2)

The apparent takeaway from this:

For an average baseline character, having a Prying 2 tool will go from succeeding most of the time to it being just shy of a coin flip.
As expected, results are unchanged if you're using a regular crowbar.
Superhuman characters can completely ignore crowbar quality, both before and after the file change.

chaosvolt · 2021-09-04T16:33:49Z

Marking as WIP for now and closing, as discussion on the BN discord has led to the suggestion of tinkering with some partial JSONization of prying. Will likely take a while.

JSONizing this is looking to be more complex than expected, and I still wanted to has out the desired balance of it first.

chaosvolt · 2021-09-04T20:52:04Z

I started to look into what would be needed to overhaul this, but with the makeshift crowbar PR affected by this PR, along with the fact that I need to hash out the balance of prying behavior either way, it seemed like it would be better to fix it now and JSONize later, than to leave it broken while I stumble my way through implementing the overhaul.

Coolthulhu · 2021-09-05T10:49:41Z

That door break function sure is ugly.
I'd rather have the JSONization before this. This PR doesn't offer much improvement by itself, but it comes with extra copypaste to maintain when things change.

Also, take results of breaking from terrain type and "ALARMED" from a flag, not hardcode.

chaosvolt · 2021-09-05T16:53:12Z

It'll take more time to JSONize things and I'm still not yet certain how well it'll go, but will try I guess. Gonna be out and about today however.

chaosvolt · 2021-09-12T00:50:16Z

Closing for now to work on this but I'm not very confident in this JSONization working out well.

SzQ1 · 2021-09-16T01:24:55Z

I'm believing in you!

chaosvolt · 2021-09-18T20:04:42Z

So I've started work on unhardcoding it, there's likely some typos here and there but the biggest sticking point seems to be:

    const std::function<bool( const tripoint & )> f = [&allowed_ter_id,
    &allowed_furn_id]( const tripoint & pnt ) {
        if( pnt == g->u.pos() ) {
            return false;
        }
        const ter_id ter = g->m.ter(pnt);
        const auto furn = g->m.furn(pnt);

        const bool is_allowed = false;
        if( has_furn( p ) && furnid.pry.pry_quality != -1 ) {
            pry = &furnid.pry;
            pry_furn = true;
            return is_allowed;
        } else if( ter( p ).obj().pry.pry_quality != -1 ) {
            pry = &ter( p ).obj().pry;
            return is_allowed;
        }
    };

Specifically the first bit, the const std::function<bool( const tripoint & )> f = [&allowed_ter_id, &allowed_furn_id]( const tripoint & pnt ) section is just about unreadable to me.

I can see very clearly that I need to eliminate the last vestige of the old allowed_ter_id and allowed_furn_id system, but what it's expected to look like to still work the same way it used to is a bit harder for me to figure out.

Saicchi · 2021-09-27T12:31:16Z

This might help: CleverRaven/Cataclysm-DDA#51778

It uses activity_data_common which was created to facilitate this process of jsonization, but it's not needed. You might see a use for it after checking thr hacksaw, boltcut, oxytorch iuse functions.

chaosvolt · 2021-09-27T14:51:01Z

I'd have to very thoroughly check what all nessecary code changes preceded this. I'm not really the right person for this, hence why I really wanted to fix the numbers first so we'd at least have decent behavior player-side, THEN worry about JSONization.

That was something I actually had accomplished and ready for review before I was talked into trying something I don't even remotely have the experience to handle.

Coolthulhu · 2021-09-28T11:50:52Z

Specifically the first bit, the const std::function<bool( const tripoint & )> f = [&allowed_ter_id, &allowed_furn_id]( const tripoint & pnt ) section is just about unreadable to me.

It's a definition of a lambda function. In order:

std::function<bool( const tripoint & )> - it's a function, that returns bool and takes a tripoint reference. This block could probably be replaced with auto, since lambdas are often autoed.
f - the function is saved into a local variable f
[&allowed_ter_id, &allowed_furn_id] it will capture allowed ter/furn ids as references, meaning it can change them. If there was no &, it would capture copies of them. You don't need those here, you aren't using them in the function.
( const tripoint & pnt ) - the function takes a tripoint reference, as mentioned above (but again). This part can't be autoed.

ter( p ).obj().pry.pry_quality != -1

You already have a ter object in the scope. It's not a function, but an id. IDs are a cata thing, not C++ thing. You can access its object with one of:

ter.obj().pry...
ter->pry...

Both of the above are equivalent.

pry.pry_quality != -1

It would be better if you checked the id for whether it is null. Like this:

if( ter->pry.the_id_of_the_replacing_terrain ) { ...

This works because ids have a defined cast to bool. If set to null, they are cast to false, if set to something not null, they cast to true.

chaosvolt · 2021-09-28T15:29:28Z

Alright, I'll need to tinker with it when I get back later today. Thank you.

chaosvolt · 2021-10-05T16:53:11Z

Okay, finally starting to look at this, I'm...hmm. Trying to make sense of this but the example provided doesn't seem like it would actually fit in the function. It's a lot to go over.

Fix compile errors Finish up pry_result loading Add pry_result checking Add JSON for previously hardcoded prying results

Fix-up prying until builds + works

chaosvolt · 2021-10-21T01:18:09Z

With thanks to @OvenBaker it now seems to compile without errors, though still showing errors of unknown nature on the checker. Going to test it some more and make sure the JSON matches up with what I had planned, then hopefully it'll be ready soonish.

Coolthulhu · 2021-10-22T13:32:28Z

though still showing errors of unknown nature on the checker

The ones with "reorder" mean that you need to have the same order in constructor as in the structure.
For example, if you have:

struct weird_order {
  weird_order() : first(0.0), second(1), third("") {}
  int second;
  double first;
  std::string third;
};

The warning will happen, because in the constructor, it goes first->second->third, but in the struct, second is the first field.
You just need to move the fields so that they appear in the same order in both.

Saicchi · 2021-10-22T14:41:49Z

For breaking the terrain / furniture on a failed prying attempt, you could use the bash function with a guaranteed chance of success, which uses the byproducts and effects of the "bash" field.
https://github.com/CleverRaven/Cataclysm-DDA/blob/718579c81958791c7ffcae5576864f22969c36b7/src/activity_actor.cpp#L4872-L4876

chaosvolt · 2021-10-22T16:25:14Z

Think I've got it organized right now. I am getting errors on compile still, but it also still compiles, I suspect I might've not missed that last time I tested it.

For breaking the terrain / furniture on a failed prying attempt, you could use the bash function with a guaranteed chance of success, which uses the byproducts and effects of the "bash" field.

That'd be useful but I'd rather get at least something actually working without errors before I get lost in yet more code changes I'm not really prepared to tackle. This PR has already suffered massive scope creep ever since I was talked into trying to JSONize the prying function instead of getting the actual balance working right first like I was trying to do.

Coolthulhu · 2021-10-22T16:33:22Z

if( pry->alarm = true ) { - use == to compare. In C++, single = means "assign".

chaosvolt · 2021-10-22T16:39:23Z

Well, that makes it no longer successfully open a build though. :/

Coolthulhu · 2021-10-22T16:42:59Z

no longer successfully open a build

?

chaosvolt · 2021-10-22T16:46:02Z

As in it goes from "successfully compiles but with errors" to "won't compile"

Coolthulhu · 2021-10-22T17:33:39Z

src/mapdata.cpp

+    difficulty( 1 ), noise( 0 ),
+    alarm( false ), breakable( false ),
+    new_ter_type( ter_str_id::NULL_ID() ), new_furn_type( furn_str_id::NULL_ID() ) {}
+break_ter_type( ter_str_id::NULL_ID() ), break_furn_type( furn_str_id::NULL_ID() ),


You have to move the {} to the end and make sure that commas match.
A comma between each field name, but no comma before the constructor body (empty, so it's just the {}).

chaosvolt · 2021-10-22T18:07:43Z

Now it looks like I can finally get to eyeballing the JSON implementation in the fix that was sent to me.

Increase effect of prying quality on prying difficulty

c5feb85

Coolthulhu self-assigned this Sep 4, 2021

chaosvolt changed the title ~~Increase effect of prying quality on prying difficulty~~ [WIP] Increase effect of prying quality on prying difficulty Sep 4, 2021

chaosvolt closed this Sep 4, 2021

Implement some ideas for current balancing

ab6426e

JSONizing this is looking to be more complex than expected, and I still wanted to has out the desired balance of it first.

chaosvolt reopened this Sep 4, 2021

chaosvolt changed the title ~~[WIP] Increase effect of prying quality on prying difficulty~~ Increase effect of prying quality on prying difficulty Sep 4, 2021

chaosvolt changed the title ~~Increase effect of prying quality on prying difficulty~~ Increase effect of prying quality on prying difficulty, door damage chance Sep 4, 2021

Coolthulhu removed their assignment Sep 5, 2021

chaosvolt closed this Sep 12, 2021

First part

a54854c

chaosvolt added 2 commits September 17, 2021 20:10

Get dat struct existin'

84eef0a

Start work on implementation

7af2479

chaosvolt reopened this Sep 18, 2021

chaosvolt marked this pull request as draft September 18, 2021 20:02

Gareth Saul and others added 5 commits October 19, 2021 13:26

Fix-up prying until builds + works

dd0f7c2

Fix compile errors Finish up pry_result loading Add pry_result checking Add JSON for previously hardcoded prying results

Merge pull request #1 from OvenBaker/prying-code-stuff

88fd1fb

Fix-up prying until builds + works

Merge branch 'upload' into prying-code-stuff

674d67b

Lintin' 'n' stylin'

d6262c4

Moar style

6f84f42

chaosvolt added 2 commits October 22, 2021 10:50

Minor fixes

07c93dd

Whoops

e18eb11

Smolfix

ef0d385

Coolthulhu reviewed Oct 22, 2021

View reviewed changes

chaosvolt added 2 commits October 22, 2021 12:43

Now it should be unfucked

2c37510

Stylin'

0015d69

chaosvolt added 2 commits October 22, 2021 13:43

Update the JSON

c1eef79

Update JSON_INFO.md

307c62e

chaosvolt changed the title ~~Increase effect of prying quality on prying difficulty, door damage chance~~ Increase effect of prying quality on prying difficulty, JSONize prying_results Oct 22, 2021

chaosvolt marked this pull request as ready for review October 22, 2021 19:57

Fix some redundant boolean stuff

27a8cad

Coolthulhu self-assigned this Oct 23, 2021

Coolthulhu merged commit 888bb31 into cataclysmbnteam:upload Oct 23, 2021

chaosvolt deleted the prying-code-stuff branch October 23, 2021 15:11

chaosvolt mentioned this pull request Oct 27, 2021

Fix prying breakage roll, implement busting doors #1098

Merged

chaosvolt mentioned this pull request Feb 7, 2023

Allow prying coffins with makeshift crowbar #2330

Merged

Increase effect of prying quality on prying difficulty, JSONize prying_results #837

Increase effect of prying quality on prying difficulty, JSONize prying_results #837

Uh oh!

Conversation

chaosvolt commented Sep 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

Uh oh!

Coolthulhu commented Sep 3, 2021

Uh oh!

chaosvolt commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaosvolt commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaosvolt commented Sep 4, 2021

Uh oh!

chaosvolt commented Sep 4, 2021

Uh oh!

Coolthulhu commented Sep 5, 2021

Uh oh!

chaosvolt commented Sep 5, 2021

Uh oh!

chaosvolt commented Sep 12, 2021

Uh oh!

SzQ1 commented Sep 16, 2021

Uh oh!

chaosvolt commented Sep 18, 2021

Uh oh!

Saicchi commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaosvolt commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Coolthulhu commented Sep 28, 2021

Uh oh!

chaosvolt commented Sep 28, 2021

Uh oh!

chaosvolt commented Oct 5, 2021

Uh oh!

chaosvolt commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Coolthulhu commented Oct 22, 2021

Uh oh!

Saicchi commented Oct 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chaosvolt commented Oct 22, 2021

Uh oh!

Coolthulhu commented Oct 22, 2021

Uh oh!

chaosvolt commented Oct 22, 2021

Uh oh!

Coolthulhu commented Oct 22, 2021

Uh oh!

chaosvolt commented Oct 22, 2021

Uh oh!

Coolthulhu Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

chaosvolt commented Oct 22, 2021

Uh oh!

Uh oh!

chaosvolt commented Sep 2, 2021 •

edited

Loading

chaosvolt commented Sep 3, 2021 •

edited

Loading

chaosvolt commented Sep 3, 2021 •

edited

Loading

Saicchi commented Sep 27, 2021 •

edited

Loading

chaosvolt commented Sep 27, 2021 •

edited

Loading

chaosvolt commented Oct 21, 2021 •

edited

Loading

Saicchi commented Oct 22, 2021 •

edited

Loading