Make it possible to specify that a block should be destroyed/dropped when replaced by another block#2990
Make it possible to specify that a block should be destroyed/dropped when replaced by another block#2990DellieDelta wants to merge 5 commits intoPixelGuys:masterfrom
Conversation
… .replaceability = .destroy
…replaceability = .drop
| const Replaceability = enum { | ||
| none, | ||
| destroy, | ||
| drop, |
There was a problem hiding this comment.
We might as well
| const Replaceability = enum { | |
| none, | |
| destroy, | |
| drop, | |
| const Replaceability = enum { | |
| no, | |
| yes, | |
| yesWithDrop, |
There was a problem hiding this comment.
In my opinion the original names are clearer and more extensible. For example if you want to add a fourth mode in the future that does something very different
There was a problem hiding this comment.
- Future proofing doesn't work. A simple example of extension would be to add support for different drop than usually is dropped when block is broken. That would require you to use union instead of enum, a major refactor either way.
- Similar pattern was already established in
rotation.RotationMode.CanBeChangedInto
There was a problem hiding this comment.
Yeahhh, you're right. Next to yes or no I don't see it getting a "maybe" loll, unless we go to a more complex structure anyway. That makes it more like a property with subproperties kind of, but with enums you can just make it flat
| _degradable[size] = zon.get(bool, "degradable", false); | ||
| _selectionRule[size] = zon.get(SelectionRule, "selectionRule", .always); | ||
| _replaceable[size] = zon.get(bool, "replaceable", false); | ||
| _replaceability[size] = zon.get(Replaceability, "replaceability", .none); |
There was a problem hiding this comment.
Its a property, it should be adjective, not a noun
There was a problem hiding this comment.
I see it more like a mode of replacement, then a noun makes more sense I think
|
So, overall I am not sure if this is flexible enough. What if instead we would support multiple Also I see either |
|
Interestingg, you'd almost need like an onReplace callback where you can say that it should drop an item, or at least some way to do that within onUpdate. It might be a bit overkill for now, but on the other hand you don't want to introduce a configuration like yesWithDrop only to remove it later again anyway |
Progress towards #1558
It's a little bit of a workaround because we don't have something like BreakBlock or ReplaceBlock yet instead of UpdateBlock, but it should be easy enough to pull apart when the time comes
Most relevant changes are in the first commit :)