HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Adding Abilities to Cards Dynamically in a Trading Card Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dynamicallycardscardaddingabilitiesgametrading

Problem

I have figured out how to pass functions around in Lua, and I am using the technique of inserting functions into tables in order to dynamically assign functions to the cards in a trading card game. The functions are assigned when the Card is first created, but more could easily be added later. I am still new to Lua so I would love to hear about any syntax errors I am making, or if the way I am doing anything might be dangerous in the future or violates Lua best practices.

The basic idea is that each Card has a unique ID, and each ID has a table of strings associated with it. The Card first gets this table of strings from the CardFunctionsList script, then passes them to the FunctionsList script which returns a table of the functions themselves. Then the card can run any function that has been loaded in this way, and could pass the function along to another card if need be. When called, the card passes itself into the function, and the function modifies it.

There are some problems with this approach. I do not like having to use tables full of strings in CardFunctionsList.lua, but I do not see a better way to do it. I originally just used numbers, but the readability was worse compared to this version. But if there are any typos the functions will not be loaded. Also, I think it might be better to have just one script instead of two to return the functions table, but it seems cleaner to have it separated this way. I am not sure about this. Finally, I am struggling with the names CardFunctionsList and FunctionsList, but I can't think of anything better that makes sense.

Card.lua

```
local Card = {}

function Card:new(cardSuit, cardType, cardId)

local instance = setmetatable({}, { __index = self })

instance.suit = cardSuit
instance.id = cardId
instance.type = cardType

--these will be assigned by a script that is not yet complete
instance.health = 0
instance.strength = 0

--these are the functions of the card
inst

Solution

This part of your code:

for name,func in pairs(functionTable) do
    if name == abilityName then
        amountToIncrease = 1
        func(self,amountToIncrease) --self because the card passes itself into the function
    end
end


Can be greatly simplified simply by checking if the index you are looking for exists or not:

local ability = functionTable[abilityName]
if ability then
    amountToIncrease = 1
    ability(self,amountToIncrease) --self because the card passes itself into the function
end


Even though the code for assigning strength and health to a card is not complete, it seems like you are assigning functions/abilities on one place, assigning strength/health at another place... And I'm not so sure about the methods to assign a card some abilities that you add when you create the card. It seems to me like it would be better to not call the addTaunt method but instead to assign the card taunt through the constructor. You seem to be grouping properties together instead of cards. You're defining all functions for all cards at one place, will you then define all strengths and healths for all cards at another place? It would be better to "group by card".

Right now you are mapping cardId --> function name in CardFunctionsList:functionNamesForId(id) and then you are mapping functionName --> Actual function. When I started refactoring this to skip the name I realized that you really need it that way, because of the doAbility function. A comment about that would be helpful so that I don't try refactoring it again :)

One thing I noted about the functions/abilities thingy though is that your FunctionsList only contains one thing: instance.functions. Why not simply use the instance directly?

FunctionsList:functionForName always returns a table. I don't see the usefulness of this. You can make it return a single function and violà, it will have reduces table nesting.

I managed to re-write your FunctionsList code to this:

function FunctionsList:new(functionNames)
    local instance = {}
    self:loadFunctionsToReturn(instance, functionNames)
    return instance
end

function FunctionsList:loadFunctionsToReturn(tabl, functionNames)
    for key,name in ipairs(functionNames) do
        tabl[name] = self:functionForName(name)
    end
end

function FunctionsList:functionForName(name)
    --even though the table key looks like plain text, it will accept a string to access it
    if       name == "increaseHealth" then return increaseHealth
      elseif name == "increaseStrength" then return increaseStrength
      elseif name == "addTaunt" then return addTaunt
      elseif name == "addCharge" then return addCharge
    end
end


And then you of course have to make the related changes in Card.lua, primarily the doAbility function which I wrote like this:

function Card:doAbility(abilityName)
    local ability = self.abilityList[abilityName]
    if ability then
        amountToIncrease = 1
        ability(self,amountToIncrease) --self because the card passes itself into the function
    end
end


A bit related to your question is this:


The basic idea is that each Card has a unique ID, and each ID has a table of strings associated with it.

When I read that I instantly thought entity-component-system, which works a little like this. Your code isn't really ECS though, but I just wanted to throw this out there.

Code Snippets

for name,func in pairs(functionTable) do
    if name == abilityName then
        amountToIncrease = 1
        func(self,amountToIncrease) --self because the card passes itself into the function
    end
end
local ability = functionTable[abilityName]
if ability then
    amountToIncrease = 1
    ability(self,amountToIncrease) --self because the card passes itself into the function
end
function FunctionsList:new(functionNames)
    local instance = {}
    self:loadFunctionsToReturn(instance, functionNames)
    return instance
end

function FunctionsList:loadFunctionsToReturn(tabl, functionNames)
    for key,name in ipairs(functionNames) do
        tabl[name] = self:functionForName(name)
    end
end

function FunctionsList:functionForName(name)
    --even though the table key looks like plain text, it will accept a string to access it
    if       name == "increaseHealth" then return increaseHealth
      elseif name == "increaseStrength" then return increaseStrength
      elseif name == "addTaunt" then return addTaunt
      elseif name == "addCharge" then return addCharge
    end
end
function Card:doAbility(abilityName)
    local ability = self.abilityList[abilityName]
    if ability then
        amountToIncrease = 1
        ability(self,amountToIncrease) --self because the card passes itself into the function
    end
end

Context

StackExchange Code Review Q#61028, answer score: 6

Revisions (0)

No revisions yet.