CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.65k stars 4.18k forks source link

[Magiclysm] Summoned items don't have length #64198

Closed Terrorforge closed 1 year ago

Terrorforge commented 1 year ago

Describe the bug

Despite having a longest_side specified in their item definitions, items that are summoned via magic have no length. This leads to absurd situations such as being able to put a "great sword of judgement" (same size as a zweihander) in your backpack.

Attach save file

Trivially reproducible

Steps to reproduce

  1. Learn and use any spell that summons an item

Expected behavior

Items have length, even when conjured magically.

Screenshots

No response

Versions and configuration

Additional context

No response

GuardianDll commented 1 year ago

Interestingly, but its not true, its something else my game contain "longest_side": "70 cm", for stormhammer, but despite this, spawning it using debug shows it still has no length image

Terrorforge commented 1 year ago

Hrm. But there seems to be nothing else connecting them? And it's happening to at least items in ethereal_items.json and the sword of judgment in the Crusader spell file. But not every summoned item - the pocket sun has a length, despite having zero volume. Something wrong with the materials, maybe?

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

Rewryte commented 1 year ago

Done some testing on this. The item::length() function will give a length of 0mm for all items that are made of a liquid material. As concentrated mana is considered a liquid, any items made of it will have 0 length. By changing the stormhammer to steel, I was able to summon one with the correct length of 70cm.

GuardianDll commented 1 year ago

And liquid it because of what? This exact bug occured when hardened mana got rigid:false boolean, not sure how it's related to liquid materials

Rewryte commented 1 year ago

Sorry spoke too soon. It's the "soft": true property. Turning that off in the concentrated mana material will make item length behave properly.

GuardianDll commented 1 year ago

I mean i know it, it was done in #63795, specifically to avoid summonned items slow you down. I guess the issue that soft property has a wrong name, as it actually means liquid

Rewryte commented 1 year ago

Ah, I wasn't aware of that PR. Liquids and soft materials share the same check, hence my initial confusion.

if( made_of( phase_id::LIQUID ) || ( is_soft() && is_container_empty() ) ) { return 0_mm; }

Splitting out the conditions show that ( is_soft() && is_container_empty() ) is returning true. But even if we can modify is_container_empty() to return false for non-containers, further down the function is_soft() is checked again.

units::length max = is_soft() ? 0_mm : type->longest_side;

So unfortunately I can't see how to give concentrated mana items length without making changes to every other soft item in the game.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.