Itemstack details #79
Conversation
|
Link #2, implements MetaDataRef:equals(other) |
| if wear <= 65535 then | ||
| self._wear = wear % 65536 | ||
| return true | ||
| else | ||
| self.clear() | ||
| return false | ||
| end |
There was a problem hiding this comment.
Is this updated engine 5.5 behavior? (yeah previous wear behavior was bit weird but kinda useful)
I'm not certain, I'll take a look. If I remember, the reference I was looking at just casts the value as uint16.
There was a problem hiding this comment.
It looks like this is how it's been done since at least 0.4.17.1
There was a problem hiding this comment.
Are you confusing this with "add_wear"?
There was a problem hiding this comment.
Ah actually it is same behavior but just like engine allows destroying items by setting invalid wear?
Should prob just add some simplified strict mode to throw errors, probably duping assert as strict_assert or something for easy use (and custom error behavior handling)
There was a problem hiding this comment.
Now wondering why I've even done that as it only affects values > 65536 and those will kill stack anyway.... 🤷 well, looks way better now so just have to add assertion thing.
I think I could try to look into that a bit tomorrow, would be nice to get this merged soon anyway.
There was a problem hiding this comment.
I'm not sure what you're talking about w.r.t. simplified strict mode or throwing errors?
Strict mode means that error is raised when something is done that does not seem clearly intended behavior like destroying tool stack by setting invalid value for it. That is also application specific thing as sometimes it is meant to be destroyed while sometimes not, would however like to have assertion near actual call that causes potentially unwanted behavior for this specific case.
Not sure how this case should be exactly handled, so far was handled with straight assertion probably just because that happened to be only use case I needed back then...
There was a problem hiding this comment.
Correct me if I'm wrong, but destroying the stack by setting the value above 65535 is the intended behavior. You used the tool too much, now it's broken. The assertion that was there previously was wrong.
There was a problem hiding this comment.
Which is why clarified with "That is also application specific thing as sometimes it is meant to be destroyed while sometimes not".
True that it is kind of wrong and should probably have been handled different way in tests from beginning, use case for that assertion is with indestructible rechargeable power tools.
It could have been done better with more specific assertions in test set by verifying that stack is still valid but... well, that was not done... but then early error also proved to be useful as when tool stack got accidentally destroyed it eventually caused error on completely irrelevant part of mod and that's reason why "would however like to have assertion near actual call that causes potentially unwanted behavior for this specific case"
There was a problem hiding this comment.
Probably needs way to mark item def as destructible/indestructible through "side effects" and global configuration for default behavior... I think that would work best here.
In particular, support for itemstack metadata serialization in the itemstring.