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

[RDY] Improved firearm accuracy mod #22731

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
6 participants
@Firestorm01X2
Copy link
Contributor

commented Jan 11, 2018

Imagine what if experienced character in cataclysm could be able to reliable hit target on 30 tiles with sniper rifle. Now you can stop imagine this. Let me introduce new small mod.

Forum topic:
https://discourse.cataclysmdda.org/t/improved-firearm-accuracy-mod/14785

Mod features:

  • This mod significantly incrases accuracy of ranged wepon based on player marksmanship and gun skill;
  • Now expirinced player can actually shoot and hit long range targets reliable;
  • Mod uses same effect as Targteting CBM but scales based on player character skills;
  • Targeting CBM gives addtitonal bonus to accuracy with this mod;
  • Original system still in place, mod just modifies it by scaling dipersion;
  • At marknmanship 3 and gun skill 3 player character get same bonus as if he has Bionic targeting. At marknmanship 6 and gun skill 6 as player get twice effect.

Showcase using police sniper with stats 8/8/8/8. Rifle: Remington with scope. Distance 30:

Marksmanship 3 rifles 3 (private with training):
r_3_3

Marksmanship 6 rifles 6 (competent sniper):
r_6_6

Marksmanship 9 rifles 9 (veteran sniper):
r_9_9

With mod sometimes actual range of even good hit may be higher than reported in wepon stats. It is because the way of calculationg range of even good hit.

Firestorm01X2 added some commits Jan 11, 2018

Improved firearm accuracy mod
Improved firearm accuracy mod
styling
styling

add( "IMPROVED_RANGED_ACCURACY", "world_default",
translate_marker( "Improves accuracy of ranged weapons" ),
translate_marker( "Improves accuracy of ranged weapons." ),

This comment has been minimized.

Copy link
@Night-Pryanik

Night-Pryanik Jan 11, 2018

Member

This description doesn't really tell us anything useful. You should write it consistent with most other options, something like "If true, ranged shots will have a much higher chance to reliably hit targets at a great distance".

@@ -1573,6 +1573,15 @@ dispersion_sources player::get_weapon_dispersion( const item &obj ) const
dispersion.add_range( std::max( vol - get_skill_level( skill_driving ), 1 ) * 20 );
}

if( get_option< bool >( "IMPROVED_RANGED_ACCURACY" ) )
{
double cbmBonus = ( has_bionic( bionic_id( "bio_targeting" ) ) ) ? 2 : 0;

This comment has been minimized.

Copy link
@Night-Pryanik

Night-Pryanik Jan 11, 2018

Member

Parentheses set before has_bionic is redundant.

Styling and descritption
Styling and descritption

@Firestorm01X2 Firestorm01X2 changed the title Improved firearm accuracy mod [RDY] Improved firearm accuracy mod Jan 11, 2018

@kasanryukin

This comment has been minimized.

Copy link

commented Jan 11, 2018

That isn't a mod, that's a rewrite of the firearms code, which Kevin is already working on. Mods are .json isolated, you've branched the code out to create a new distro based on your firearms work.

However, Kevin or whomever else reads this, what he's done is actually not a bad way of overhauling firearms. It fits close with the data I've been trying to collect on firearm ranges and accuracy comparisons based on the US Army Marksmanship Program, and Law Enforcement Firearms Qualifications Tests, since those two gave the broadest spectrum of available data comparing the beginner, the moderate, the advanced, and the elite among firearms owners. I'd suggest testing the commits with a variety of weapons at various ranges and skill levels, but this might not be a bad idea on fixing firearms, since the above graphic for a 9/9 sniper with no aiming seems like a good base point for somebody that dedicated to marksmanship.

(DruidNiam on the forums btw)

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2018

That isn't a mod, that's a rewrite of the firearms code, which Kevin is already working on. Mods are .json isolated, you've branched the code out to create a new distro based on your firearms work.

Not always. Look at "Filthy_Morale" and "novitamins" for example.

@kasanryukin

This comment has been minimized.

Copy link

commented Jan 11, 2018

Filty_Morale is hard coded into the source code, The reason it's in the mod category and has a folder and modinfo.json is so people can disable it by telling the game to ignore the tags in the mod. Your mod is a code merge pure and simple. I'm not saying it's a bad one. Just call a spade a spade.

Edit: The vitamin mod does the same thing. Hard coded switch to turn off nutrition and modinfo to tell the game to ignore the now unused vitamin tags.

@kasanryukin

This comment has been minimized.

Copy link

commented Jan 11, 2018

Also the images shows a max range of 60, has that always been there and I'm a dope and haven't noticed that before?

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2018

Also the images shows a max range of 60, has that always been there and I'm a dope and haven't noticed that before?

It is max rifle range. It is in its stats. Thing is that for loong time there are wepons that potentialy can reach everything you see but player himself never will be able to hit on that range.

@kasanryukin

This comment has been minimized.

Copy link

commented Jan 11, 2018

Yeah the RivTech Sniper Rifle can target out to maximum range, but you need bonkers perception under the current code to actually hit anything even at 10/10 skill.

{
double cbmBonus = has_bionic( bionic_id( "bio_targeting" ) ) ? 2 : 0;
double skillEffect = 1 - std::min( ( double( get_skill_level( skill_gun ) + double( get_skill_level(
obj.gun_skill() ) ) ) ) / 2 + cbmBonus, double( MAX_SKILL ) ) * 0.0833;

This comment has been minimized.

Copy link
@Night-Pryanik

Night-Pryanik Jan 11, 2018

Member

The formula is too hard to understand imo (with all these lots of parentheses). Maybe move the first parameter of std::min into its own variable, like cbmBonus above?

Also, where does this 0.0833 magic number came from?

This comment has been minimized.

Copy link
@kasanryukin

kasanryukin Jan 11, 2018

I dunno, I could use a calculator and follow the formula just fine. The closer to 10/10 skill level I inputted the closer to .0833 I got. I don't know what the result of the formula equates to when multiplied by the dispersion.add_multiplier gets affected. On the forums he stated that the dispersion total can end up lower than the hard cap of 45.

Edit: which I think was a part of the point of the mod.

refactoring and comments
refactoring and comments
double cbmBonus = has_bionic( bionic_id( "bio_targeting" ) ) ? 2 : 0;// insalled cbm work as bonus skill levels
double perSkillMult = 0.0833;// = 0.25/3 to get one bio_targeting effect every 3 levels of avgSkill
double avgSkill = ( double( get_skill_level( skill_gun ) + double( get_skill_level(
obj.gun_skill() ) ) ) ) / 2;

This comment has been minimized.

Copy link
@Night-Pryanik

Night-Pryanik Jan 11, 2018

Member

Redundant external parentheses.

Also (just in case if I understand you correctly), do you mean a) double( X + double ( Y ) ) / 2,
or b) double( X + Y ) / 2?

refactoring and removing redudant code
refactoring and removing redudant code
@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 11, 2018

There's no tests or even description of what it's supposed to do other than "increase accuracy", so there's no way to evaluate if it's working as intended.
It invalidates everything about how ranged balance is handled in the game right now.
It's a bunch of random hard coded hacks to dispersion values that WILL break if we change anything else about firearm handling.
There's no rationale for why these adjustments happen, you somehow can compensate for how accurate your gun is with "skill", there's no way for that to work. (yes, bio_targeting does the same thing, and bio_targeting is ridiculous, but that's scifi computer-aided aiming, not how shooting works)

We have a set of accuracy tests for firearms, if they have problems, bring those problems up and/or propose alternate thresholds, but ad-hoc adjustments to accuracy calculations are a non-starter.

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2018

Ok no
So, even in mod state it is not allowed here. Few lines of code, activated by mod. Mod.
Then I should ask: is there plans to fix current ranged balance properly?

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Jan 12, 2018

@kevingranade While I do agree that this mod is a bunch of arbitrary hacks to make ranged combat more appealing to one personal opinion, this is a mod after all, and the effects won't affect the core game with mod turned off.

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2018

Exactly. About that mod:

  1. It is mod.
  2. Code wise it is short and localized, so it may be removed easily and do not mess the code.
  3. It is stable. As long as CBM targeting will not break whole targeting system - it is safe.
  4. Effect is explainable and understanble.

We have a set of accuracy tests for firearms, if they have problems, bring those problems up and/or propose alternate thresholds, but ad-hoc adjustments to accuracy calculations are a non-starter.

Issue for tests exists already:
#22211

Current tests does not satisfy ranged balance, proposed by Coolthulhu.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jan 13, 2018

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Jan 13, 2018

Push the weapon dispersion adjustment down to the json definitions, this is a purely mechanical change. Then you can add a mod to set weapon dispersion to whatever you want.

If you DO want to adjust how skills and accuracy interact, you need to find a way to make that moddable in json, simply injecting new hardcoded behavior doesn't work.

Guess what I am planned while doing this:
#22402

That was way to expose something quickly to JSON without adding it to world options. Also this constant manager allows easy modding of that constants. Current world options support only boolean constants. Imagine - just JSONize something and just use get_constant(). No more changing in code. No need to add world options for everything.

You didn't listen and simply closed it. But that thing should have been really useful. Especially it should have been useful for modding.

Next thing I planned - to put constants it JSON and load that using new constant manager.

I still can reimplement that. I still have source code. I mean really - lets put balance things to JSON. It even should help in testing. I bet you rebuilded source thousands of times to get current balance.

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2018

@tyrael93

This comment has been minimized.

Copy link

commented Mar 16, 2018

Could you bundle the new version as a mod with lua edits? Some of us are using bright nights or heavily modded versions of the game already. Or non-windows builds. Or non-tiles builds.

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2018

Could you bundle the new version as a mod with lua edits? Some of us are using bright nights or heavily modded versions of the game already. Or non-windows builds. Or non-tiles builds.

Sadly it can't be achieved with lua.
About builds- it is my first atemp, so I decide to start with windows builds.

Also it is temporay solution until Kevin finish gun rebalance. Sadly it may take a lot of time. So here it is- alternative.

About mods: it should be compatible with everything. It is based on main repository after all.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2018

This has nothing to do with Lua.

@tyrael93

This comment has been minimized.

Copy link

commented Mar 17, 2018

Really? Damn, I thought it could do anything regular code changes could. Modding with .json really is limited.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2018

Lua is limited to exposed CPP functions.

@Firestorm01X2 Firestorm01X2 deleted the Firestorm01X2:accuracymod2 branch Jul 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.