sandstone-mc / sandstone

Sandstone | Next Generation Framework for Minecraft
https://sandstone.dev/
MIT License
173 stars 16 forks source link

Either remove `tag.add`, or rename `tag.add` to `tag.push` and implement `tag.unshift` #153

Closed GrantGryczan closed 1 year ago

GrantGryczan commented 2 years ago

Only allow the tag's values to be interfaced via tag.values rather than trying to copy an arbitrary subset of array methods as shorthands which honestly are not necessary in the first place. I think this option would be preferable due to the fact that it is simpler and removes redundant patterns.

But if you are against that, then if a tag is meant to be interfaced with directly, I think it would be better if it could be interfaced similarly to an array. Why call the method "add" if it specifically inserts it at the end? Tags (or rather, function tags in particular) are ordered, so it doesn't make sense to abstract adding things to it as generally "adding", which doesn't specify where it's being added, rather than being pushed to the end. set.add makes sense, but tag.add doesn't, since tags act like arrays (which are ordered), not sets (which are unordered). Things should be able to be added to the end (tag.push) or to the beginning (tag.unshift). But I think this option is unideal because:

  1. There is redundancy between tag.values methods and tag methods.
  2. It could lead to a slippery slope of other array methods being needed, as well as inconsistency from using some methods on tag but others on tag.values when they aren't supported on tag.

(Note that either way, my tag.has suggestion (#116) is still necessary as it is very complicated to check whether a value is in a tag, unlike these array methods which aren't necessary as they aren't even really abstracting anything, only renaming/slightly shortening.)

(Also note this would make #110 unnecessary.)