helix
helix copied to clipboard
NebulousCloud
Account for characters without an inventory
Account for characters, such as NPC's, who have a character but not an inventory, this causes an index nil error.
There shouldn't be instances of characters without an inventory, NPCs don't need to be characters.
There shouldn't be instances of characters without an inventory, NPCs don't need to be characters.
Shouldn't, but I've seen servers where this is an issue, hence the PR
I did send a pull request to COTZ to include a pcall for their shop system, where the NPC is a character without an inventory. One of the items uses :GetCharacter and :GetPosition for an item description in the GPS.
NPCs with valid characters isn't really an intended use case whatsoever though
NPCs with valid characters isn't really an intended use case whatsoever though
Hey, I don't write these plugins. All I know is this is a completely reasonable patch without any downsides besides possibly encouraging poor practice?
Reminder that it's not just NPC's that might not have an inventory, :GetInventory may return nil for actual players, who knows what server owners do these days...
I'd much prefer it error for something you shouldn't be doing than add more validity checks to be honest with you.
I'd much prefer it error for something you shouldn't be doing than add more validity checks to be honest with you.
Still, it's not strictly erroring for something you shouldn't be doing, it just doesn't account for the inventory being nil.
Perhaps there's a really valid reason for a character to not have an inventory, not that I know of one.
I don't think we need to be writing features and fixes into the framework because people are using features of the framework incorrectly. Characters are simply just intended to have inventories, there's nothing more esoteric to it than that.
This wouldn't be the only case across the codebase where this edge-case is handled. This would have more merit if it fixed all of those edge cases, but on it's own, it feels like a band-aid to a problem that could eventually sprawl out of control.
To improve the request I recommend instead of fixing this instance only, look across the rest of the codebase for instances of this behavior and lock it all up into one neat tiny bundle.
Though, there really, really, shouldn't be a character without an inventory. If a character shouldn't have an inventory, it'd probably be far easier for everyone if a hook was used to prevent any items from entering the inventory.
Just going to stick this here since I have seemed to confuse you all when I stated this issue was simply localised to misusing the system. I was just using the most basic example I could think of.