Generate: remove tag "-" #3036
Generate: remove tag "-" #3036
Conversation
github-actions
bot
added
affects: core
| cleaned_value.difference_update(new_value) | ||
| elif isinstance(new_value, list): | ||
| for element in new_value: | ||
| cleaned_value.remove(element) |
There was a problem hiding this comment.
.remove() only seems to remove the first occurrence; do we want to remove all?
>>> a = ['a', 'a', 'b']
>>> a.remove('a')
>>> a
['a', 'b']I would think that for my use-cases, we should remove all occurrences (just because multiple triggers with overlapping effects are more likely to stack the same item in the list multiple times if it doesn't cause a difference). This means that lists that formerly didn't have a semantic meaning behind duplicates can now behave differently under triggers if they have duplicate entries. While I think it's possible for a yaml writer to make use of that behaviour somehow, I think that's well into dark yaml territory and I'd imagine removing all would be the more intuitive behaviour. Then again, I haven't played a game where duplicate list entries had any semantic meaning, they might be out there.
Whichever behaviour we go with (remove first or remove all), it should be documented somewhere because the question will either be asked or it will be an unknown effect that will torment yaml writers getting too fancy.
There was a problem hiding this comment.
Since writing this I've dived more into options and learned the options API supplies both OptionSet and OptionList. Given .removed() should always remove all occurrences in a set (because there can only be one occurrence), I think the behaviour of only removing the first element in a list is more reasonable.
However, lists and sets are written the same way in the yaml, so yaml writers may have no indication which structure they are working with. Behaving differently for different structures introduces some sharp corners unless the world maintainers are very good about communicating what option is what.
The final behaviour should still be documented somewhere.
Exempt-Medic
added
the
is: enhancement
* Generate: introduce Remove, similar to Merge * make + dict behave as + for each value --------- Co-authored-by: Zach Parks <[email protected]>
* Generate: introduce Remove, similar to Merge * make + dict behave as + for each value --------- Co-authored-by: Zach Parks <[email protected]>
* Generate: introduce Remove, similar to Merge * make + dict behave as + for each value --------- Co-authored-by: Zach Parks <[email protected]>
What is this fixing or adding?
add "-" operation similar to the "+" operation
I expect this to be one of the first feature requests once "+" exists.
There is an opiniated/questionable function in here though when it comes to dict handling.
{"a": 1} + {"a": 2} -> {"a": 2}
is the current + behaviour, however, - does:
{"a": 1} - {"a": 2} -> {"a": -1}
I considered it out of scope to touch + behaviour to act as a per-key-value + in this PR, but maybe for consistency that should be done?
How was this tested?
only minimally so far
If this makes graphical changes, please attach screenshots.