Currently, I manage my custom inventory system with a table in the user’s data. I’ve seen many other users suggest that you have a custom class to manage inventories similar to the following examples: Inventory.new(), Inventory.Get(), Inventory.AddItem(), and Inventory.RemoveItem().
What would be better?
My current inventory system keeps two copies of the player’s data. A client version and the master server version (the real data). To make it so the client can see their changes to their inventory appear instantly, I have the changes to their data be processed right away so the UI doesn’t feel laggy. But when that happens the server is also sent this information to do sanity checks (the client also has sanity checks but with the client data instead). Once the server approves of the changes, it sends the client a data update with the information that the client takes then overrides the current client data with that (this might be troubled by lots of lag).
What’s wrong with this system? Should I change anything? Is there a better solution?
If you have only one inventory that you care about (The player) then your system isn’t too bad. Though let’s say you have a chest or storage object in your game, technically that’s an inventory as well. So having an OOP style with it allows you to create inventories for players AND objects reusing the code.
I mean using a class has its perks, but whether or not its really beneficial is just evaluating if this new system is better then your old system.
I guess the some of the perks of a custom class is:
the behavior and how you get attributes,
it is a lot cleaner to write,
gives structure to your code.
Methods as well are good ways to make sure your code is efficient - you can even use getters and setters to get / set your values. Inheritance and overriding is a nice feature to have from OOP as you can gain attributes from superclasses meaning you don’t have rewrite code and you can get properties from this super class. You can also then use these methods to treat data the same in your server code although the background code is different, for example :Pronoun() could return different things for a male / female but then in your code you could simply call the method and the value data is returned differently.
But the biggest issue with your system isn’t going to be your class methodology, but your attitude to how data is represented:
I think like a lot of people forget what a GUI actually is and how they are applied, its a graphical user interface in your case on a thin client like most games - not a fat client where the client works independent from the server. Your client is meant to represent changes on the server to the client, you shouldn’t even need to perform a sanity check on the client because irregardless of what they do on their side it will have no effect on the server side. The server is in control, the client is just there to take user inputs.
I mean someone can change their health on the client, does that show on the server - no lol. You would be better adapting your system to just never trust the client, if the client requests to do an action (because they have met the client criteria) then simply authenticate - does the client have the ability to do this? Any data stored should be held for visual representation and not as a core part of your code - this is because core parts on the client can easily be changed. I only store a copy of the money for example just for visual representation - yes they could change the value but in reality if they try to make the server recognize this new value it will not work.
The reasoning behind the client data table is to have changes like dropping items (the amount of items going down) instantly so the experience is more fluid. The reason the sanity checks are in place on the client is because it’s checking the same things the server would but on the client’s data table to prevent just a normal user from dropping items they don’t have any more. Developing with it, the system can be a hassle: you need to have two (sometimes) slightly different checks to accomplish one task.
TLDR: The client data is there to make the UI non-laggy for those who have high ping.
Now, the real question is if it’s even worth the fluid non-laggy UI to setup this semi-annoying system. What are your thoughts on that?
I mean I don’t store the client data, I store core representational data. My UIs come out fluid and values get passed back and forth through remote functions and remote events whenever the changes are necessary. For example you said about:
Why would the client be dropping the items themselves, this could easily be changed to "a client can make requests as much as they want but the server is only going to drop said item IF they possess said item. Events and such get queued, so if they already have a request in a queue to drop said item then the next request would be treated differently.
When your just representing the data its actually a lot smoother and memory efficient, because your own UI coding is what determines how smooth / not smooth something is.
But would I drop your current system for this class system?
No, its not necessary and its not worth sacrificing your current work just so you can access the certain benefits.
Would I switch to data representation instead of fat client?
Yes, its memory efficient and clients only have the data they need to represent themselves.
I’ll probably end up leaving the client checks. It’s hard to explain, but it did offer great benefits for creating really nice fast UI because both the server and client where doing checks on their own versions of the data (most of the time they’d be in sync of course). I have other methods of displaying whether or not somebody can drop something, for example, with my notification system. The one thing I would miss about the system is the information of whether or not they could drop something was handy to have on the client without having the request the server. A handy use of this would be when you click submit on dropping an item: if the client knows that it can’t drop enough items, then tell the player, don’t close the menu, and don’t request the server.
It’s just a little too much extra stuff to warrant doing so. Shame.