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

feat(content,port): corpses decay into items, use existing harvest entries to actually implement feature, fix off-map decay behavior #4751

Conversation

chaosvolt
Copy link
Member

@chaosvolt chaosvolt commented Jun 7, 2024

Purpose of change

This aims to port over a feature recently added to DDA that'd been on my wishlist for a while, allowing corpses to decay into bones when left alone. Along the way, I went from having to actually do the JSON implementation myself to finding aw ay to make it automatically affect all butcherable monsters without needing to touch JSON.

Some cleanup and consistent edits to harvest JSONs were cooked up along the way, and subsequently moved to #4755 now that they're officially unrelated to this PR's changes.

Describe the solution

C++ changes:

  1. In item.cpp, added handling of decaying corpses to item::process_internal, and definition of item::get_corpse_mon to item.cpp and item.h. Also set it to not do this with corpses that're about to zombify into something, so it doesn't break that.
  2. In map.cpp and map.h, ported over map::handle_decayed_corpse from the source PR, converted to behave with the item identity behavior, and with some additional improvements. Most importantly, it now uses the monster's harvest entry directly, setting the items to use the corpse's birthday so that any perishable materials in the harvest yields will rot away correctly, leaving behind only bones, sinew, etc. This way we don't actually have to implement a million fucking decay entries in the JSON. It also has a fix for the potential issues of spawning bionics or bionics groups on doing this, the latter if not forbidden would cause issues due to interpreting an itemgroup as an item and erroring.
  3. Also in map.cpp, added a separate check for corpses being valid to decay into map::actualize, right before the call to remove_rotten_items. Reason is, just shoving the function call to handle_decayed_corpse into map::remove_rotten_items was causing it to implode with an access violation I couldn't figure out the cause of, but inserting a (somewhat hacky I feel, but functional) check for handling this earlier seems to make it happy at the expense of most likely making Scarf cry.
  4. Per request by Viss/Lamadus, went ahead and implemented the planned followup to this PR: decaying corpses can now potentially drop any CBMs or other corpse components that generated on them, at the 10% chance you get as the bare minimum dissection success chance. For CBMs it'll just roll to decide whether to drop a burnt-out bionic instead, while non-CBM components simply drop at a 10% rate.

JSON changes:

  1. Set it so that bone/chitin always appears after sinew on harvest entries, mostly for aesthetic reasons so that corpses that decay away leave the most relevant leftover visible on top.

Describe alternatives you've considered

If desired, the chance to return CBMs from decaying corpses can either be postponed for a followup PR, nerfed even lower than the minimum success rate of dissection (if we feel we want to incentivize going for rock-bottom dissections of corpses about to rot, personally I feel using the same odds will probably encourage the player to waste their time less often in favor of going for better tools/skills), or just not done at all if people strongly object to having it.

Testing

  1. Compiled and load-tested.
  2. Killed my started NPC and advanced time, confirmed he rots away into just sinew and human bones, leaving no meat or such. The bones now show on top of the stack too, instead of the sinew like before.
  3. Spawned in a bobcat and did this as well, confirmed they yield sinew and the correct type of bones.
  4. Spawned in a feral human, confirmed they zombify if waited out properly, but in both compiled test build and in release 2024-05-30 just advancing time has a high chance to make the body vanish instead.
  5. Spawned a broken cyborg, confirmed it doesn't attempt to drop cbms or pull the itemgroup from bionic_group as an item.
  6. Started in next summer scenario and warped around until I found a dead body map extra, confirmed the bodies there had left bones and sinew.
  7. Started anew, murdered my starter NPC, teleported well away from the shelter, advanced time to summer, then teleported back. Body correctly rotted away to bones.
  8. Teleported to a mine in next summer scenario as well, no implosions despite those typiucally having bodies downt here.
  9. Teleported all around a city in said next summer start with no crashes.
  10. Debug murdered an anthill, warped out, advanced a year, warped back in. No implosions, just lots of chitin and sinew left on the ground.
  11. Stress-tested the off-map decay feature by having the entire refugee center murdered, warping off-map, advancing time a year, then warping back. Everyone turned into bones without issues or any noticeable lag spikes.
  12. Spawned in what turned out to be 2008 moose and debug killed them. That alone lagged out my game for about a minute. Warped out, advanced time a year, warped back in. The game chugged for about the same amount of time decaying that many bodies at once but didn't explode.
  13. Tested with a cyborg after implementing the corpse component drops, first testing before random chance was introduced. Obtained a corpse that scanning and dissection-testing confirmed had an arm alloy armor CBM and a light battery in it. Warped away, advanced time, warped back in to find it had indeed dropped both items as expected.
  14. Retested after adding the addition that clears the non-sterile fault from on-bionic drops, light battery correctly no longer spawned faulty.
  15. Did some further tests after implementing the randomization, correctly got it dropping a burnt-out bionic most of the time, sometimes an intact CBM, sometimes also the battery on the side.

Next summer drug deal location:
image

Ant leftovers after debug-murdering them and warping back in after advancing time by a year:
image

A fresh example showing how bones are now at the top of the item stack instead of sinew:
image

End result of refugee center test:
image

Unlimited Moose Works:
image

And finally, screenshot from the cyborg corpse components testing, showing a CBM and non-CBM corpse component successfully dropping as intended:
image

Additional context

PR source: Corpses can decay into bones, etc., by @RenechCDDA: CleverRaven/Cataclysm-DDA#74328

In the future I'd also like to have it attempt to dump CBMs and other items generated on a corpse when it rots away, likely at the minimum of 10% chance so the player is still encouraged to harvest them themselves, but that can be a separate PR.

Checklist

@github-actions github-actions bot added docs PRs releated to docs page src changes related to source code. JSON related to game datas in JSON format. labels Jun 7, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Contributor

autofix-ci bot commented Jun 7, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@chaosvolt chaosvolt changed the title feat (port): corpses decay into items, actually implement in JSON feat (port): corpses decay into items, actually implement in JSON Jun 7, 2024
@chaosvolt chaosvolt changed the title feat (port): corpses decay into items, actually implement in JSON feat (content, port): corpses decay into items, actually implement in JSON Jun 7, 2024
@scarf005 scarf005 changed the title feat (content, port): corpses decay into items, actually implement in JSON feat(content,port): corpses decay into items, actually implement in JSON Jun 7, 2024
chaosvolt and others added 3 commits June 6, 2024 21:51

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@chaosvolt chaosvolt marked this pull request as ready for review June 7, 2024 02:54
@chaosvolt chaosvolt changed the title feat(content,port): corpses decay into items, actually implement in JSON feat(content,port): corpses decay into items, use existing harvest entries to actually implement feature, misc harvest sanity-checking Jun 7, 2024
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this adds new methods in item and map class directly, which we should generally avoid without good reason (check #3200 for details). not sure for items, but for map it seems like the method added could have been a separate free function. could we extract the method into regular function here?

@chaosvolt
Copy link
Member Author

Maybe? Not sure where best to put it.

Also, discovered a critical issue with this: bodies don't actually drop their harvest results if they rotted away while out of the reality bubble. Also covers bodies that spawn in map extras and the like when visited long after zero day.

@chaosvolt
Copy link
Member Author

Tested DDA to see if I could replicate it, I can't even actually get bodies to properly recognize that it's been a year so they should've already rotted like I can in BN, and also managed to get it to error about not recognizing the harvest entry for the decay value I gave a zed for testing, so DDA's version of it is for now a complete write-off.

Notifying @RenechCDDA since this is a DDA error with their version of the feature:
image

@chaosvolt
Copy link
Member Author

chaosvolt commented Jun 7, 2024

So, root problem here in particular is corpse removal off-map appears to be handled by map::remove_rotten_items: https://github.com/cataclysmbnteam/Cataclysm-BN/blob/main/src/map.cpp#L7264 while item::process_internal seems to mainly be used for in-map spawns.

Problem is, dumping a call to handle_decayed_corpse into map::remove_rotten_items seems to explode, specifically because it really does not like the idea of calling add_item_or_charges at a tripoint that's off in parts unknown. I'm not sure if this is the reason why the source PR retained its weird hack of "get abs then localize later" (doubt, as no call to the function was injected into any apparent counterpart to map::remove_rotten_items in the PR) or if the solution is something else.

@github-actions github-actions bot removed docs PRs releated to docs page JSON related to game datas in JSON format. labels Jun 7, 2024
Father I crave violence
@chaosvolt chaosvolt changed the title feat(content,port): corpses decay into items, use existing harvest entries to actually implement feature, misc harvest sanity-checking feat(content,port): corpses decay into items, use existing harvest entries to actually implement feature, fix off-map decay behavior Jun 8, 2024
@chaosvolt chaosvolt marked this pull request as ready for review June 8, 2024 07:23
@github-actions github-actions bot added the JSON related to game datas in JSON format. label Jun 8, 2024
@chaosvolt
Copy link
Member Author

Two problems discovered:

  1. Off-map decay is beefing up and actually spawning its item twice.
  2. Something about copper wire specify seems to be spawning whole stacks instead of charges.

Off-map decay of a cyborg, with a not-yet-commited addition that lets CBMs spawn:
image

On-map decay, working correctly except for big stacks of wire:
image

@chaosvolt chaosvolt marked this pull request as draft June 8, 2024 19:32
There's one part I hate about this and that's the way I have to move stuff out of the `remove_with()` function to make it work.

Sort of works but non-count_by_charges harvest materials don't stack when spawning for some reason?
@KheirFerrum KheirFerrum force-pushed the corpses-decay-into-bones-but-actually-implement-it branch from f3e4798 to 7f3ce80 Compare June 9, 2024 06:34
joveeater
joveeater previously approved these changes Jun 9, 2024
Copy link
Collaborator

@joveeater joveeater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Adding items into a temporary vector and then handling them at the end to avoid memory problems is fine and it's something we do in other places.

@chaosvolt
Copy link
Member Author

Aight, retested. Everything still seems to be good except for the copper wiring thing:
image

This only affects pretty much a single monster currently, and I plan to defer the question of harvesting bionics this way for a later PR, so I'll go ahead and mark as ready and just see if I can get time to fix this weird case.

@chaosvolt chaosvolt marked this pull request as ready for review June 9, 2024 17:34
scarf005
scarf005 previously approved these changes Jun 10, 2024
@chaosvolt
Copy link
Member Author

Apologies to Scarf for also dismissing the appoval, but I figured out what seems to be a viable fix for the "copper wires from cyborgs spawn way more than intended on decay due to being multiplied by default stack" issue:
image

@chaosvolt
Copy link
Member Author

And at request of @VissValdyr during discussion in the BN server, I went ahead and added the "decay can potentially drop corpse components" idea in this PR instead of in a follow-up, since is a fairly basic addition. Uses the same odds and general behavior as the bare-minimum chance of obtaining corpse components when dissecting.

Lowering it to something like 5% instead of 10% is a potential option if we feel it more balanced to make "at least try even though you know the success rate is at rock bottom currently" still worth attempting.

@chaosvolt chaosvolt merged commit 1d53969 into cataclysmbnteam:main Jun 14, 2024
13 checks passed
@chaosvolt chaosvolt deleted the corpses-decay-into-bones-but-actually-implement-it branch June 14, 2024 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants