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

Update Aftershock dead survivor loot item group #62236

Closed
wants to merge 1 commit into from

Conversation

BrettDong
Copy link
Member

Summary

None

Purpose of change

#61575 forbids corpses containing items. This change breaks an item group defined in the Aftershock mod. This is causing errors in basic build test in almost all recent pull requests.

Describe the solution

Update the broken item group to spawn the corpse alongside with the loot items.

Describe alternatives you've considered

Testing

./tests/cata_test --mods=dda,aftershock "~*" no longer errors.

Additional context

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding Mods: Aftershock Anything to do with the Aftershock mod Spawn Creatures, items, vehicles, locations appearing on map <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Nov 15, 2022
@BrettDong BrettDong requested a review from Fris0uman November 15, 2022 17:15
@Fris0uman
Copy link
Contributor

It should spawn correctly in game though, this 393f630 fixed the error when loading the game at least. Maybe we need to do somethign similar for the test run?

@BrettDong
Copy link
Member Author

Item group consistency check also calls put_into_container() to put loot items in the corpse. If it works fine in game then the test should also pass.

@Fris0uman
Copy link
Contributor

So you mean it doesn't actually work in game?

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Nov 15, 2022
@BrettDong
Copy link
Member Author

Yes, the error also appears when I try to create a character in a world with Aftershock mod:
image

@mqrause
Copy link
Contributor

mqrause commented Nov 16, 2022

I'm not 100% sure how these item groups with corpses work. But as far as I understand they're supposed to create corpses that "wear" stuff with the worn stuff containing other items spawned in the group. Not sure if this fix will still produce that result or if it will spawn a corpse next to some other items. If it's the latter, then the real issue is more like #59873: some item group doesn't have appropriate container items to contain all the additional things it spawns.

@Fris0uman
Copy link
Contributor

Okay so I don't know where I messed up my test. Still I think it would be better to find a c++ fix so that the items are still put into corpses as if they were wearing it rather than droping it next to them. Plus in vanilla the mapgened corpse can revive and they'll keep their loot if they have it inside the pocket

@mqrause
Copy link
Contributor

mqrause commented Nov 16, 2022

Looking into it a bit more, the issue seems to be even still different:

if( ctr.can_contain( *it ).success() ) {
const item_pocket::pocket_type pk_type = guess_pocket_for( ctr, *it );
ctr.put_in( *it, pk_type );

item::can_contain will also check all nested pockets of the container, however item::put_in can only put things directly in the container. That's why it sometimes tries to insert things into a corpse the normal way instead of forcing it in or putting it into an appropriate container. So that needs to be sorted out one way or the other.

@BrettDong BrettDong closed this Nov 16, 2022
@BrettDong BrettDong deleted the afs_corpse branch November 16, 2022 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mods: Aftershock Anything to do with the Aftershock mod Mods Issues related to mods or modding Spawn Creatures, items, vehicles, locations appearing on map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

afs_corpse_old fails on item group test
3 participants