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

Support creating items from item groups with spells #64600

Merged
merged 2 commits into from
Apr 5, 2023

Conversation

mqrause
Copy link
Contributor

@mqrause mqrause commented Mar 26, 2023

Summary

Features "Support creating items from item groups with spells"

Purpose of change

Documentation was saying this is a thing already when it absolutely isn't.
I also want to reduce stray uses of item::in_its_container to have an easier time changing its behavior.

Describe the solution

  • refactor spell_effect::spawn_ethereal_item to support SPAWN_GROUP flag
  • remove WITH_CONTAINER flag because the same thing can be done with itemgroups
  • UI has no proper way to display item group results, but I don't want to get too sidetracked with this

Describe alternatives you've considered

Testing

Changed a spell to use an item group and used it. It spawned the group (damage times), granted items were still ethereal if not comestible/permanent.

Additional context

@github-actions github-actions bot added <Enhancement / Feature> New features, or enhancements on existing <Documentation> Design documents, internal info, guides and help. [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON [Markdown] Markdown issues and PRs Mechanics: Enchantments / Spells Enchantments and spells json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Mar 26, 2023
@GuardianDll
Copy link
Member

FYI EoC has such ability now, widly used in xedra mod

@mqrause
Copy link
Contributor Author

mqrause commented Mar 27, 2023

So you want to tell me I can just completely remove the "spawn_item" spell effect cause there are better ways to do it?

@GuardianDll
Copy link
Member

Sure not, having more ways to do a thing is always better

@mqrause mqrause marked this pull request as ready for review March 28, 2023 14:59
@mqrause mqrause requested a review from KorGgenT as a code owner March 28, 2023 14:59
@Fris0uman
Copy link
Contributor

Sure not, having more ways to do a thing is always better

No, it's confusing and brittle. Having one good centralized way to do things is better code wise

@GuardianDll
Copy link
Member

right now to spawn loot group from spell, you should make a spell, a loot group, and additional EoC that will spawn a loot group
why not get rid out of additional stuff, if we can afford it? its like saying "let's get rid out of damage dealing in spells, because we have guns and effects already, that can do the same"

@mqrause
Copy link
Contributor Author

mqrause commented Mar 29, 2023

Just to be clear, there's over 100 spells that spawn items and I definitely won't rewrite all of them to use EOCs to remove the spawn_item spell effect. That'd go far beyond what I wanted to achieve with this, which is to clean up a bit of code for some other stuff I'm working on.

@kevingranade kevingranade merged commit de40ba8 into CleverRaven:master Apr 5, 2023
@mqrause mqrause deleted the spell_item_group branch April 5, 2023 14:41
Maleclypse pushed a commit to Maleclypse/Cataclysm-DDA that referenced this pull request Apr 6, 2023
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 [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. <Enhancement / Feature> New features, or enhancements on existing [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs Mechanics: Enchantments / Spells Enchantments and spells
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants