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

Item groups don't spawn blacklisted items #79341

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mqrause
Copy link
Contributor

@mqrause mqrause commented Jan 25, 2025

Summary

Bugfixes "Item groups don't spawn blacklisted items"

Purpose of change

In #79285 it came to light that blacklists weren't regarded with item group spawns.

Describe the solution

Add containers to item removal in item modifiers.
Also add containers to consistency check in item modifiers.
There's potential (and need honestly) for test cases here, but I want to see if it breaks existing tests before I work on that.

Describe alternatives you've considered

Testing

Ran tests with this PR on top of #79285 and [force_load_game] --mods=dda,generic_guns.

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jan 25, 2025
@SurFlurer
Copy link
Contributor

SurFlurer commented Jan 25, 2025

AFAIK item blacklist thing gets processed in void Item_factory::finalize_item_blacklist()? It tries to remove blacklisted items from every Item_spawn_data, which includes modifier ( container thingy ), Single_item_creator, and Item_group.

Maybe my reasoning for a9bcfa1 isn't as solid as I thought, and it broke the blacklist logic?

Update: No, generic guns still can prevent hwp modules from being spawned in item group robofac_gun_kit that includes an anonymous item group, so it should be ok.

@mqrause
Copy link
Contributor Author

mqrause commented Jan 25, 2025

Oh, might actually be only modifiers are affected? I guess I do need to start with test cases after all.

@mqrause mqrause marked this pull request as draft January 25, 2025 16:59
@SurFlurer
Copy link
Contributor

SurFlurer commented Jan 26, 2025

I can only find that the container_item of a Item_spawn_data doesn't get blacklisted. I can't test whether this is the problem, since I dunno the exact test case that blacklists fail to take effect.

@mqrause
Copy link
Contributor Author

mqrause commented Jan 26, 2025

contents-item is what the mentioned PR tripped over. You can spawn the group ar15_22 with Generic Guns enabled to confirm.

@mqrause mqrause force-pushed the no_blacklist_spawn branch from a97cab4 to e086f4d Compare January 26, 2025 18:00
@github-actions github-actions bot added the <Bugfix> This is a fix for a bug (or closes open issue) label Jan 26, 2025
@mqrause
Copy link
Contributor Author

mqrause commented Jan 26, 2025

It appears to be a much simpler fix than initially anticipated, since the infrastructure for everything is already there, although I added another fix for another similar oversight.

Still need tests for this, though.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 26, 2025
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) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants