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

Kris knife won't fit into a sheath #22686

Closed
aeoo opened this Issue Dec 31, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@aeoo

aeoo commented Dec 31, 2017

Game version: 0.C-25592-gb478bedc45

Operating system: Linux

Tiles or curses: Tiles

Mods active:

DDA
No Fungal Monsters
Classic Roguelike Classes
StatsThroughSkills
Urban Development
Bionics Systems Mod
Boats
Craftable Gun Pack
Bright Nights
Folding Parts Pack
Filthy Clothing
More Buildings
Classes and Scenarios Mod
Disable NPC Needs
Simplified Nutrition
Alternative Map Key

Expected behavior

Sheath claims it will accommodate up to 0.75 liters, so it should do just that. Kris knife is 0.75 liters and a sorcerer class from one of the mods starts with kris sheathed in a 'sheath', which makes sense.

Actual behavior

Once the sorcerer's kris is unsheathed from its sheath, it can no longer be resheathed, and the description for kris doesn't list 'sheath' as one of its containers.

Steps to reproduce the behavior

Should be trivial, just start the game with a sorcerer from I believe "classic roguelike classes" mod. Unsheathe the kris and try to resheathe it.

@BorkBorkGoesTheCode

This comment has been minimized.

Show comment
Hide comment
@BorkBorkGoesTheCode

BorkBorkGoesTheCode Dec 31, 2017

Contributor

The ankle holster says it will only accept items with volume 0.66+. I think floating point math may be involved.

Contributor

BorkBorkGoesTheCode commented Dec 31, 2017

The ankle holster says it will only accept items with volume 0.66+. I think floating point math may be involved.

@aeoo

This comment has been minimized.

Show comment
Hide comment
@aeoo

aeoo Dec 31, 2017

I'm talking about a 'sheath', which is a bit larger than an ankle holster.

It occupies 'leg' layer 'waist'.

If you mean even something with .75 might not be precisely that much, then I hear that.

aeoo commented Dec 31, 2017

I'm talking about a 'sheath', which is a bit larger than an ankle holster.

It occupies 'leg' layer 'waist'.

If you mean even something with .75 might not be precisely that much, then I hear that.

@BorkBorkGoesTheCode

This comment has been minimized.

Show comment
Hide comment
@BorkBorkGoesTheCode

BorkBorkGoesTheCode Dec 31, 2017

Contributor

Sorry for being vague. I was mentioning that the ankle holster has weirdness as well.

Contributor

BorkBorkGoesTheCode commented Dec 31, 2017

Sorry for being vague. I was mentioning that the ankle holster has weirdness as well.

@aeoo

This comment has been minimized.

Show comment
Hide comment
@aeoo

aeoo Dec 31, 2017

Ah, now I get it, thanks. I looked at JSON and everything is specified in integers in JSON. For 'sheath' max_volume is 3, and 'kris' is volume 3.

I think I found the problem. It's in the definition of "kris":

"SHEATH_SWORD"

Kris is not a sword. I think the problem is in the flag documentation file, where it says something like "knives are 2 or less", which makes no sense. It should say "knife are 2 or less, with a few exceptions." It shouldn't cause people to be dogmatic about the number 2, but I think it does.

So either the definition should be relaxed, or kris should be made volume 2, because it is a knife and not a sword.

Based on how the sorcerer class is defined, everything used to work in the past. I don't see any reason why kris should no longer fit into a knife sheath.

aeoo commented Dec 31, 2017

Ah, now I get it, thanks. I looked at JSON and everything is specified in integers in JSON. For 'sheath' max_volume is 3, and 'kris' is volume 3.

I think I found the problem. It's in the definition of "kris":

"SHEATH_SWORD"

Kris is not a sword. I think the problem is in the flag documentation file, where it says something like "knives are 2 or less", which makes no sense. It should say "knife are 2 or less, with a few exceptions." It shouldn't cause people to be dogmatic about the number 2, but I think it does.

So either the definition should be relaxed, or kris should be made volume 2, because it is a knife and not a sword.

Based on how the sorcerer class is defined, everything used to work in the past. I don't see any reason why kris should no longer fit into a knife sheath.

@aeoo

This comment has been minimized.

Show comment
Hide comment
@aeoo

aeoo Dec 31, 2017

The kris should be sheathed in a scabbard.

Not so. Kris is a knife. It should not be sheathed in a scabbard. I know thanks to the game definitions it can be, but that's not correct.

https://en.wikipedia.org/wiki/Kris

It's definitely only a knife.

aeoo commented Dec 31, 2017

The kris should be sheathed in a scabbard.

Not so. Kris is a knife. It should not be sheathed in a scabbard. I know thanks to the game definitions it can be, but that's not correct.

https://en.wikipedia.org/wiki/Kris

It's definitely only a knife.

@BorkBorkGoesTheCode

This comment has been minimized.

Show comment
Hide comment
@BorkBorkGoesTheCode

BorkBorkGoesTheCode Dec 31, 2017

Contributor

That would make sense and fix the issue.

Contributor

BorkBorkGoesTheCode commented Dec 31, 2017

That would make sense and fix the issue.

@aeoo

This comment has been minimized.

Show comment
Hide comment
@aeoo

aeoo Dec 31, 2017

http://www.baliblog.com/travel-tips/bali-travel/balinese-culture/the-sacred-kris-indonesian-traditional-knife.html

But I also found some slight counter-evidence. I found a store that sold "kris swords" with 20 inch blades. Also the blog above says the blade length is "highly variable."

One thing that irks me is that if it's volume 3 and the sheath holds volume 3, but because of a flag it won't work. I think volume should be enough to determine if something matches or not.

On the other hand, if we want to get technical and talk about shape and other properties, the kris require their own special holsters anyway.

Another way to fix this issue would be to outfit the sorcerer with a scabbard.

Apparently 'sheath' was good enough in the past. I'm in favor of allowing kris to be considered a knife. But if the powers that be decide this isn't ideal, then at least the sorcerer class should come with a set of matching equipment. Right know it comes with kris in a sheath so once unsheathed, you get a useless sheath and kris that cannot be stored on the body. :)

aeoo commented Dec 31, 2017

http://www.baliblog.com/travel-tips/bali-travel/balinese-culture/the-sacred-kris-indonesian-traditional-knife.html

But I also found some slight counter-evidence. I found a store that sold "kris swords" with 20 inch blades. Also the blog above says the blade length is "highly variable."

One thing that irks me is that if it's volume 3 and the sheath holds volume 3, but because of a flag it won't work. I think volume should be enough to determine if something matches or not.

On the other hand, if we want to get technical and talk about shape and other properties, the kris require their own special holsters anyway.

Another way to fix this issue would be to outfit the sorcerer with a scabbard.

Apparently 'sheath' was good enough in the past. I'm in favor of allowing kris to be considered a knife. But if the powers that be decide this isn't ideal, then at least the sorcerer class should come with a set of matching equipment. Right know it comes with kris in a sheath so once unsheathed, you get a useless sheath and kris that cannot be stored on the body. :)

@kevingranade

This comment has been minimized.

Show comment
Hide comment
@kevingranade

kevingranade Jan 1, 2018

Member
Member

kevingranade commented Jan 1, 2018

fungusakafungus added a commit to fungusakafungus/Cataclysm-DDA that referenced this issue Apr 1, 2018

Add a test for CleverRaven#22686
Tests whether profession-specific items in holsters can be put back once
removed.

fungusakafungus added a commit to fungusakafungus/Cataclysm-DDA that referenced this issue Apr 1, 2018

Add a test for CleverRaven#22686
Tests whether profession-specific items in holsters can be put back once
removed.

fungusakafungus added a commit to fungusakafungus/Cataclysm-DDA that referenced this issue Apr 1, 2018

Add a test for CleverRaven#22686
Tests whether profession-specific items in holsters can be put back once
removed.

@fungusakafungus fungusakafungus referenced this issue Apr 1, 2018

Merged

Add a test for #22686 #23349

2 of 2 tasks complete

fungusakafungus added a commit to fungusakafungus/Cataclysm-DDA that referenced this issue Apr 8, 2018

kevingranade added a commit that referenced this issue Apr 26, 2018

Add a test for #22686 (#23349)
* Add a test for #22686

Tests whether profession-specific items in holsters can be put back once
removed.

* Added a matrix job for mods testing

* Give sorcerer/sorceress a scabbard instead of a sheath

so kris knife would fit

fixes #22686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment