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

FRAGILE_MELEE, continued. #24028

Merged
merged 7 commits into from Jun 27, 2018

Conversation

Projects
None yet
10 participants
@NotFuji
Copy link
Contributor

commented Jun 16, 2018

Continues the feature added in #23963

  • FRAGILE_MELEE now checks for the weakest component used to make the item for use in damage calculations. Crafting components now directly impact the durability of said weapons

  • Items commonly used as "handle wrappings" such as rags and leather are omitted.

  • Additional skill/stat modifiers are now correctly applied

  • Fragile items are given a further 6x bonus increase to damage chance to compensate for the apparently over-strong materials that are often weakpoints. ( Cotton, more specifically. If cotton were to have it's chip_resistance lowered to 1 from 6, function would be nearly identical as no bonus chance. ) Overall, items like the knife spear break approximately half as often as they do in mainline.

  • Upon breaking a fragile item has a chance to drop it's crafting components. Handles (the largest component by volume) are retained by the player, and the weakest item has a higher chance of being destroyed. Closes #24018

  • For fragile items with no specified crafting components, such as those naturally spawned into the world, damage chance is 6x the default value. Items are simply destroyed when broken. ( This should rarely happen if ever, FRAGILE_MELEE is meant for shoddy player-made weapons )

@Firestorm01X2

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2018

I suggest to think about possibility to use some of new behavoir by default.

@0be1isk

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2018

NOW THIS IS WHAT I'M TALKING ABOUT.
This is an example of a not so half baked spiteful idea.

@Lorith

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2018

I'll have to see how it works out ingame, but this seems like a far better solution, and should be able to work in many cases equally well.

@Xhuis

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2018

I think FRAGILE_MELEE was originally written in a quick-and-dirty way as part of an attempted nerf against knife spears, just like that other PR. Up until now, the flag has been lacking and very niche. This makes the flag more useful and should hopefully encourage additions with it that don't have to explicitly state they're not trying to nerf something.

@0be1isk

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2018

I tried playing with the half baked tag AlienZimogor hastly PRed, and the durability of the knife spear is on par with the extremely absurd weapon durability in BOTW, so I removed it from my JSON folder.
Because no one seemingly wants to/or it seems cares about the consistency, I'm working on a PR that applies the tag to most if not all "items" in the game that makes sense. For example, you can use a glass bottle or bowl as a weapon, however irl, if you were to do so, it would break instantly. This fragile tag would work well for those items.

@victorsegall

This comment has been minimized.

Copy link

commented Jun 16, 2018

Quite sensible that a component is damaged and therefore is the part referenced for damage to cause the break. This will also support variant recipes that use more or improved materials. As-is it supports the fact that existing recipes have variable ingredients that can represent consequential choices.

Reading thread for the PR #23963 shows its author being consistent (FRAGILE_MELEE because we already have DURABLE_MELEE) rather than opinionated like #24005 which simply uses the flag before it's ready to be used. #23963 was only using knife spear as an obvious example, which it is. The flag is appropriate and this PR improves it. #24018 will also improve it. It needs to be improved, as @kevingranade and others are clearly guiding @AlienZimogor to force the work to get done.

Set Zimogor's misguided motivations and language aside, because we'll have to. Could mod it out, but shouldn't be forced to.

Without looking, I'd take the bet that DURABLE_MELEE hasn't received this level of attention. It should.

@DracoGriffin

This comment has been minimized.

Copy link
Member

commented Jun 16, 2018

Looks much more well-thought out and put together. Extremely well done, Fuji.

Like victor suggested, is DURABLE_MELEE in a suitable place or would it also benefit from some of your treatment, Fuji?

@NotFuji

This comment has been minimized.

Copy link
Contributor Author

commented Jun 16, 2018

DURABLE_MELEE probably works just find as-is. There's not much that can be done to make it more interesting without completely changing how item damage works.

int weak_chip = INT_MAX;

// Items that should have no bearing on durability
const std::vector<itype_id> blacklist = { "rag",

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jun 17, 2018

Contributor

You want a std::set here. Then check it with blacklist.count( type_here ) > 0.

"leather",
"fur" };

std::vector<item> valid_components;

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jun 17, 2018

Contributor

No need to make copies of the components. You can iterate over components themselves while they're still components - you don't alter them here and only take the type and chip resistance.

_( "<npcname>'s %s breaks apart!" ),
str.c_str() );

std::vector<item> all_comps = temp.components;

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jun 17, 2018

Contributor

No need to copy the component vector here. Just iterate over the components themselves.
If you need to make a copy (for example, before wielding), it's better to explicitly copy it there.

std::vector<item> all_comps = temp.components;

for( auto &comp : all_comps ) {
int break_chance = comp.typeId() == weak_comp ? 2 : 8;

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jun 17, 2018

Contributor

I'm not sure what you're doing here.
weak_comp is the item with the highest chip_resistance. Why does this specific item get 4 times lower chance at avoiding destruction?

This comment has been minimized.

Copy link
@NotFuji

NotFuji Jun 17, 2018

Author Contributor

You have that backwards. weak_comp has the lowest chip resistance, and so is considered the "breaking point" of the weapon and has a lower chance of being recovered when the weapon falls apart.

continue;
}

if( comp.has_flag( "HANDLE" ) && !is_armed() ) {

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Jun 17, 2018

Contributor

Rather than explicit HANDLE flag, which would require a lot of marking, you could default to the largest item.
It would be correct in majority of cases and the minority could be special cased.

Requested Changes
 - Simplified blacklist with std::set.
 - Removed unnecessary copying of component list.
 - Replaced "HANDLE" flag with check for largest component by volume.
@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

ETA? (I'm going to stop asking because it might be annoying)

@ZhilkinSerg ZhilkinSerg self-assigned this Jun 27, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

I've already broken 5 knife spears, but no components appeared yet.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

I've already broken 5 knife spears, but no components appeared yet.

My bad. That only applies to debug spawned items - apparently they aren't crafted and have no components. Hand crafted spears are shattered fine.

@ZhilkinSerg ZhilkinSerg merged commit 3e28411 into CleverRaven:master Jun 27, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 23.812%
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Jun 27, 2018

@Xhuis

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2018

Yay! This makes it a compromise that's a lot less frustrating. Thank you!

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.