A or B | Efficiency Opinion

Help and Feedback Code Review
#1

Keep in mind this is a general thought about datastores & saving data, I want to know what would be more efficient and load on the datastore for general uses and not only purchases.

Greetings fellow developers, this question/thought has come upon my mind countless times in the past month, I am interested to know what you guys think of this… Between 2 Developers, we both had a different opinion on which one would be more efficient and which of these is better to save on a datastore, let’s get to the question then:

Assuming that you can’t purchase the next tool without owning the previous, which is a better choice/more efficient to use(You can only own one of each tool)
A) Save owned tools in a table:

local toolTable = {"WoodenSword","WoodenPickaxe")

B) Save only the latest tool, with a string:

local latestTool = "WoodenPickaxe"

Dan wants to get all the tools he already owns but he also wants to purchase a tool, he wants to make sure it’s not a tool he already owns. StonePickaxe is better then StoneSword, Dan wants to purchase the StonePickaxe but he doesn’t own the StoneSword… Which one would better assist him in checking if the tool he currently owns stands one before the tool he wants to purchase?

These 2 examples are ones i’ve came across:
A) =

--Variables
local Items = game.ServerStorage.Items:GetChildren()
local InventoryExample = {"WoodenSword", "StoneSword"}

--Functions
function NextItem(Inventory)
    local BestOwned, BestOwnedPrice = nil, 0
    for i, v in pairs(Inventory) do
        local ItemPrice = Items[v]:GetAttribute("Price")
        if ItemPrice > BestOwned then
            BestOwned, BestOwnedPrice = Items[v], ItemPrice 
        end
    end
    
    local x = nil
    for i, v in pairs(Items) do
        local ItemPrice = v:GetAttribute("Price")
        if ItemPrice > BestOwnedPrice then
            --update x
        end
    end
    return --NextItem
end

print(NextItem(InventoryExample))
local item = {"GoldenSword"}
if NextItem(InventoryExample) == item and Items[NextItem(InventoryExample)] < Items[NextItem(NextItem(InventoryExample))] then
  print("purchase right")
end

B) =

--Variables
local Items = game.ServerStorage.Items:GetChildren()
local LatestItem = ""
for i,v in pairs(Items) do
  Items[v] = i
end
table.sort(Items, function(e, r)
  return e:GetAttribute("Price") < r:GetAttribute("Price")
end)
--Functions
local function attemptPurchase(item)
  if Items[LatestItem] + 1 == Items[item] then
    print("purchase right")
  end
end

Thanks for reading! feel free to share your opinion.

2 Likes
#2

Option B, according to your example.

1 Like
#3

For starters here. You mention “Assuming you can’t purchase the next tool without owning the previous” > “Which is a better choice/more efficient to use” – Care to explain a little more on why that would be more efficient? Some developers say have an Egg system where they open an egg, adds a pet to a set inventory, none of that owning previous and next logic. What kind of system are you willing to implement to handle the purchasing and equipping of those pets?

ALSO, doing :GetChildren() to get the items from a folder in ServerStorage is an old method to getting the data of items players can purchase, I suggest you switch to a handy Shared module that can hold all the items and data of the items that are for purchase.

Something like this is what I use & probably most developers:

local Items = {
	["Tool1"] = {
		["Price"] = 15,
		["Damage"] = 5,
	},
	
	["Tool2"] = {
		["Price"] = 30,
		["Damage"] = 10,
	},
};

return Items;

Now down to breaking this post down:
In my opinion, I would not use any of these methods. Storing an array is pretty much pointless because you cannot get much out of it other than the items, storing the singular item is worse because how are you going to ensure they purchased all the others? To counter this, create one single dictionary stored:

local ItemData = {
    ["Item1"] = false,
    ["Item3"] = false,
    ["Item2"] = true,
};

This is ALL that gets stored in the datastore, and it provides me pretty much everything I need. You can take it a step further too and encode it for the datastore to make it even MORE efficient.

Encoded Example:

{"Item1":false,"Item2":true,"Item3":false}

Another store example:

local ItemData = {};

--// Store
function Store:Own()
	--// Own an item
	ItemData["Tool"] = false;
	
	--// do more
end

--// Checking if an item is owned
function Store:IsOwned()
	return (ItemData["Tool"] ~= nil);
end

--// Checking if an item is equipped
--// Inventory
function Inventory:IsEquipped()
	return (ItemData["Tool"]);
end

function Inventory:Equip()
	--// Equip an item
	
	--// Unequip others
	for Index, Value in pairs(ItemData) do
		if ItemData[Index] ~= "Tool" then
			ItemData[Index] = false;
		end;
	end;

	--// Equip new
	ItemData["Tool"] = true;
end

That is something I would use in one of my games, obviously I’d have 2 different modules for Store and Inventory. They both handle different things.

With the script I provided there, you can create an order from the Shared module of all stored items, find the previous and next item from there. It doesn’t get much easier than that. It’s neat, efficient and simple to read.

Storing Tables compared to storing Strings in the datastore isn’t really much of a difference unless your string is HUGE. Both your methods are a hacky workaround to attempting to make a what you call “more efficient” solution. Hacky is never good.

I do have to mention, your post is all over the place. At the top of your post you mention "Is it better to save the latest purchase in the datastore or save all purchased as a table in the datastore. You then quickly jump to “Which one would better assist him in checking if the tool he currently owns stands one before the tool he wants to purchase?” I’m a bit confused on if you’re asking two questions or …?

#4

You’ve provided an example which goes exactly the opposite of what I asked and besides that you are creating an unnecessary loop instead of rather just having a string with equipped and storing the owned tools in an array, not a table. I still stand by my opinion regardless of what you think, please do show me how you handle purchases next time.

#5

I would advise against making small “subheadings” in your code. It’s a piece of code, not an essay.

See the Roblox Lua style guide.
(This style guide is not prescriptive, but it’s good to follow.)

To answer your question, I would say B. It doesn’t actually matter, though.

Anyway, here’s my take on it:

  • Store a table somewhere with classes of tools as keys and the tools as values.
local things = {
    pickaxe = {Items.WoodenPickaxe, Items.StonePickaxe}
}
  • What the user currently has is stored as a number in a dictionary:
local inventory = {
    pickaxe = 1 -- they have a wooden pickaxe
}
  • When the user wants to buy a new pickaxe, you just increment the number.
if inventory.pickaxe < #items.pickaxe then
    inventory.pickaxe += 1
    return true -- success
else
    return false -- fail
end

This is more efficient because:

  • seeing what the user currently has is an O(1) operation

  • getting a new item is also an O(1) operation
    your NextItem function becomes a simple addition

  • it’s smaller to store in a datastore
    {"pickaxe":1,"sword":2} vs ["WoodenPickaxe", "WoodenSword", "StoneSword"]

1 Like
#6

I see where your coming from, thank you for the opinion. How do you plan on giving the inventory a number as each? I mentioned they cannot buy a new tool before owning the previous, I think you missed that one tho :thinking:. Good work,thanks for the suggestion.

#7

I want to break down this reply a little.

I’ve provided an example that directly relates to the core process of handling a Store and Inventory which was in your thread (by the way)

There is no “unnecessary loop” happening. Just one simple table stores the items the player owns, with a corresponding Bool to display the Equipped status of that item. You’re suggesting that I should have a string of the equipped AND store the owned tools in an array.

So after all that, you still decide to ask me how I handle purchases? Interesting. Ok.

Example of handling a purchase:

local Store = {};

--// For the sake of your post, we add a previous item check
function Store:OwnsItem(Item)
    local WeaponData = ...
    return (WeaponData[Item] ~= nil);
end

function Store:OwnsPrevious(Item)
    local PreviousItem = (StoreContent[Item].Order - 1);
    return (Store:OwnsItem(PreviousItem));
end

function Store:Purchase()
    local WeaponData = ...
    --// Do your necessary checks. Make sure they own it, (Checking if they own the previous) ...

    --// Set the weapon in their data
    WeaponData[ItemName] = false;

    --// Go to our inventory and equip it
    Inventory:Equip(ItemName);
end

return Store;

This is how I handle Stores and Inventories, including the PreviousItem too.

#8

The loop itself,why do all that if you could just have a string to handle it? I don’t see a sole purpose to continue with looping when you could just have a string to grab it,saving you all the work of setting multiple booleans false each time, you’ve provided me an example which is inefficient in many ways, the sole purpose of making tables is to make life easier and not harder, you constantly create loops for the code to run. This is why in my opinion you should have a separate string for storing the equipped item instead of a table then an array.

#9

I seem to be mistaken of the intentions of this post. It seems like you are telling me rather than asking me which method is better. I’m giving you my personal opinion along with code from what I personally use.

I have a module for the Store and a module for the Inventory

They both have about 4 methods, simple methods. Simple stored.
Doesn’t require me “grabbing strings” or anything like that.
The sole purpose of the store module I posted above is to make your life a lot easier, and I think most people can argue, that it does.

I’m not “constantly” creating loops for the “code to run”. It’s every-time the player purchases.
I think you’re also forgetting that storing a string in the datastore can also be costly, as I mentioned above, the more characters you use, the harder it gets for the datastore. If you are that concerned about the efficiency, storing a string of the equipped item and then somehow unpacking it to assume they own the other items is not the way to go about it. The less thinking the server has to do, the better. The server has to do quite a lot of thinking with the method you are so strongly backing up.

#10

It seems you misunderstood the point that I’m not relating anything here to equipping and the sole purpose was as a general purchase question,thank you for the opinion,I appreciate it.

#11

And they can’t.
You can only buy the next tool in the set because you only ever increment the number by one.

The number corresponds to it’s index in the things table.

To make it more clear:

local things = {
    pickaxe = {
        [1] = Items.WoodenPickaxe,
        [2] = Items.StonePickaxe
    }
}

local inventory = {
    pickaxe = 1 -- 1 is WoodenPickaxe, 2 is StonePickaxe, etc
}
#12

I did not misunderstand anything, I personally would not use any of those methods. I think you are slightly confused on the actual method I’m trying to explain.

My items are stored in a table, with the Index being the item name and the Value being the equipped status. That’s all it really needs and it doesn’t get much simpler than that. However, I mentioned you could Encode it using HttpService and then set that to a variable in the datastore if you are so against having tables being stored in the datastore.

The template for my data would look a little something like this (to help you actually visualize this):

return {
	["Gems"] = 50,
	["Money"] = 1,
	["Wins"] = 0,
	
	["Pickaxes"] = {["Item1"] = false},
	["Codes"] = {"code", "cool", "yes"},
}

As you can see, very little is being stored in the datastore itself

Bottom line, I just think the 2 methods you provided are not good to use and they are both quite frankly, a little too hacky for my liking.