Is there a more efficient way of doing this science system?

ID:2095811
 
Jun 6 2016, 6:51 pm
Code:
var/list/science = new/list("Blueprint","Fuel","Shovel","Spacepod","Spaceship")



mob
    var
        Intelligence // Intelligence Mod, effects how fast one gains knowledge.
        Knowledge // Knowledge
obj
    Items
        Grab
            verb
                Get()
                    set category = null
                    set src in oview(1)
                    if(usr)
                        if(!usr.KO)
                            Move(usr)
                            viewers() << output("<font color=gray>[name] grabs [src].","output1")
                Drop()
                    set category = null
                    set src in usr
                    loc = loc
                    step(src, dir)
                    viewers() << output("<font color=gray>[name] drops [src].","output1")






            Minerals // Players will not use a built-in currency, but can create their own with materials given to them, or create in-game trading systems.
                Copper

                Gold

                Iron

                Mythril // used for fuel, mainly.

                Stone

                Wood

            Science
                var
                    coppercost
                    goldcost
                    ironcost
                    mythrilcost
                    stonecost
                    woodcost
                    quality // quality of the object. 1% to 100%.
            
            
                Click(obj/O)
                    if(O in science)
                        for(var/obj/Items/Grab/Minerals/Copper/A in usr.contents)
                            if(A < coppercost)
                                return
                        for(var/obj/Items/Grab/Minerals/Gold/A in usr.contents)
                            if(A < goldcost)
                                return
                        for(var/obj/Items/Grab/Minerals/Iron/A in usr.contents)
                            if(A < ironcost)
                                return
                        for(var/obj/Items/Grab/Minerals/Mythril/A in usr.contents)
                            if(A < mythrilcost)
                                return
                        for(var/obj/Items/Grab/Minerals/Stone/A in usr.contents)
                            if(A < stonecost)
                                return
                        for(var/obj/Items/Grab/Minerals/Wood/A in usr.contents)
                            if(A < woodcost)
                                return
                        O = new(src.loc)
                                
                            
                Blueprint



                Fuel
                    coppercost = 0
                    goldcost = 0
                    ironcost = 0
                    mythrilcost = 100
                    stonecost = 0
                    woodcost = 0 // 1 litre of fuel is 100 mythril; 1 litre of fuel 100% quality fuel is enough to get halfway from Earth to the closest planets.
                Shovel
                    coppercost = 0
                    goldcost = 0
                    ironcost = 0
                    mythrilcost = 0
                    stonecost = 50
                    woodcost = 15
                Spacepod
                    coppercost = 550
                    goldcost = 250
                    ironcost = 375
                    mythrilcost = 250
                    stonecost = 0
                    woodcost = 0
                Spaceship
                    coppercost = 2500
                    goldcost = 1500
                    ironcost = 2000
                    mythrilcost = 1500
                    stonecost = 0
                    woodcost = 0


Problem description:

All code listed up above, I am mainly curious about the Click procedure I have, using a bunch of for statements. Is there any way I can do this differently, and have I messed up with the current method?
Jun 6 2016, 7:10 pm
Yes, instead of storing materials as an object you could store it as a number, reducing the number of object instances on the map. Seems weird to have "if o in science" when you're clicking a child of the science class. I assume there's only one instance of material in inventory? In that case, you don't need to do a whole loop, just a find.


EDIT: Also looks like you're comparing an object and a number. Huge runtime error going to pop up.
Jun 6 2016, 7:56 pm
A slightly more efficient way to do it would be to give the mob variables for coppercount, goldcount, etc, adding and subtracting from them with Get() and Drop(). Then you could just check that against coppercost, goldcost, etc instead of using all of those for statements.
Jun 6 2016, 8:11 pm
For example, 
mob
     var
          coppercount=0


          Get()
               set category = null
               set src in oview(1)
               if(usr)
                  if(!usr.KO)
                       Move(usr)
                       viewers() << output("<font color=gray>[name] grabs [src].","output1")
                       if(src.type==/obj/Items/Grab/Minerals/Copper)
                            usr.coppercount+=1
          Drop()
               set category = null
               set src in usr
               loc = loc
               step(src, dir)
               viewers() << output("<font color=gray>[name] drops [src].","output1")
               if(src.type==/obj/Items/Grab/Minerals/Copper)
                    usr.coppercount-=1


                Click(obj/O)
                     if(usr.coppercount < coppercost)
                         return
                     O = new(usr.loc)
Jun 6 2016, 8:14 pm
Some of the original code is a bit buggy or unfinished but that's the general idea.
Jun 6 2016, 9:36 pm
In response to Seteron14
That would add more unnecessary code rather than removing it. I'd need different if statements for each mineral in both drop and grabbing.
Jun 6 2016, 9:41 pm
The point of that is not to reduce the amount of code, but to reduce overhead, or the amount of excess cpu used/lag being generated.
Jun 7 2016, 12:16 am
Without *hugely* changing your code structure, here's what I'd do:
/mob
    var/list/materials = list() // List of materials this mob has collected

/obj/Items/Grab
    verb/Get()
        set category = null
        set src in oview(1)
        if(!usr.KO)
            Move(usr)
            viewers() << output("<font color=gray>[usr] grabs [src].</font>","output1")
            pickup(usr) // use pickup() to handle any object-specific picking-up functionality

    verb/Drop()
        set category = null
        set src in usr
        loc = usr.loc
        viewers() << output("<font color=gray>[usr] drops [src].</font>","output1")

    Minerals // Players will not use a built-in currency, but can create their own with materials given to them, or create in-game trading systems.
        var/material = null
        pickup(mob/user) // don't keep the material objects around if you're going to want 2000+ of them, keep a count instead
            if(material)
                user.materials[material] += 1
            else
                user << "\The [src] is useless!"
            del(src)

        Copper
            material = "copper"
        Gold
            material = "gold"
        Iron
            material = "iron"
        Mythril // used for fuel, mainly.
            material = "mythril"
        Stone
            material = "stone"
        Wood
            material = "wood"

    Science
        var/cost = list() // Cost of this science; associative list, ie cost["gold"] is either null (which acts like zero) or the amount of gold required
        var/result = null // What this science makes.

        Click()
            for(var/mineral in cost)
                if(cost[mineral] > usr.materials[mineral])
                    usr << "You don't have enough [mineral] to make that!"
                    return
            for(var/mineral in cost) // if you want the science to not cost resources, remove this for()
                usr.materials[mineral] -= cost[mineral]
            new result(loc)

        Fuel
            cost = list(mythril = 100) // 1 litre of fuel is 100 mythril; 1 litre of fuel 100% quality fuel is enough to get halfway from Earth to the closest planets.

        Shovel
            cost = list(stone = 50, wood = 15)

        Spacepod
            cost = list(copper = 550, gold = 250, iron = 375, mythril = 250)

        Spaceship
            cost = list(copper = 2500, gold = 1500, iron = 2000, mythril = 1500)


This doesn't allow dropping minerals, but that wouldn't be hard to do; just make a list on startup with:

/var/list/material_to_object

/world/New()
    ..()
    material_to_object = list()
    for(var/type in typesof(/obj/Items/Grab/Minerals) - /obj/Items/Grab/Minerals))
        var/obj/Items/Grab/Minerals/M = type // this is *not* an instance, don't use it like one!
        material_to_object[initial(M.material)] = type

/mob
    verb/Drop_Material(material in material_to_object)
        set src = usr
        if(materials[material] <= 0)
            src << "You don't have any [material] to drop!"
            return

        materials[material] -= 1
        var/type = material_to_object[material]
        new type(loc)
Jun 7 2016, 2:09 am (Edited on Jun 7 2016, 2:57 am)
In response to E52680
That would add more unnecessary code rather than removing it. I'd need different if statements for each mineral in both drop and grabbing.

No, you wouldn't. The materials should have the same parent, which means they'd inherit the same drop procedure.

I'm also going to take a minute here to criticize your pathing.

/obj/Items/Grab/Minerals


Why is grab in there? In fact, why is Items even there? I assume grab is in there because you want to emphasize that the item can be grabbed, but that can be handled in a variable. And Minerals is probably there because you want it to be grabbed? But there's a better way.

The main thinking you should have as you set type paths is this. X is a Y. Not all rectangles are squares. Not all cats are cheetahs. You want to maximize inherited behavior. If you're using minerals purely as... what is essentially money, there's no need to have them in the items tree because you will never "use" copper, you'll never "throw" mythril.

Use this tree:

/obj/Minerals
/obj/Items
Jun 7 2016, 8:53 am
While GingaNinja is right about changing the cost to a list, one thing to be mindful of is that lists designated at compile-time for an object like that are created in a hidden init proc. So anything that exists many, many times in a world (e.g. a turf, or almost every obj rather than specific types of them) should avoid such things. Here it probably doesn't matter.

You do however need copper, stone, etc. to be strings in that cost list.

Another alternative to lists is using a string and parsing it for cost, which is much easier now. You could use json_decode for that, or roll your own. That's probably not ideal, but it is another option.

Best of all is if you build your game to be expandable, and keep your crafting recipes in a simple file. But that might not be important for many kinds of games.

About your type paths, I'm a little confused why you have /obj/Items/Grab here. I'd just stick with something simple like /obj/item for all items that have the Get and Drop verbs.