World generation - Why won't it work..

ID:1548645
 
Apr 17 2014, 12:35 pm
(See the best response by Nadrew.)
Hey there, been having some trouble with my world generation system, I have tried multiple approaches to this, but so far this one 'seemed' to be the most promising on my requirements. But alas, like the other attempts, it doesn't work. Not even a little.

Code:
var/global/datum/controller/generator/map_generator/mapgenerator //Set in world.New()

datum/controller/generator/map_generator
    var/list/turflist = list() // Pre-generate this list when initializing for quicker setup.
    var/list/completedtiles = list()

    var/list/minerals = new/list(
    /turf/walls/mineral/copper,
    /turf/walls/mineral/iron
    )

datum/controller/generator/map_generator/proc/SpawnMinerals()
    world.log << "HAI THERE"
    for(var/turf/walls/mineral/M in minerals)
        M = new(locate(1, 1, 1))
        if(M)
            var/rarity = M.rarity
            var/turf/spawnloc = pick(turflist)
            while(5000)
                world.log << "While Loop"
                if(prob(rarity))
                    M = new(spawnloc)


Problem description:

It doesn't work. I can see the 'HAI THERE' message appear, but it never gets to the 'While Loop' part.
The line after the for loop was added because It was incapable of finding M.rarity (null), even when I gave it the mineral type directly.

I'm at a loss here, and could use some help.

Apr 17 2014, 12:56 pm
Best response
while(5000) means nothing.

while(variable < 5000)
    variable++


Means something.

while(5000) is pretty much saying

while nothing is 5000 do something.

The only instance where something like this works would be

while(1) which equates to while(true), which will be an infinite loop.

As for why you need to use new() inside of the for(), you can't access the variables of something that doesn't exist, a type-path is just that, a type-path, it's not an object.

You're pretty much trying to access variables that don't actually exist until you create a new() object of that type.

You should be doing

for(var/turf_type in minerals)
    var/turf/walls/mineral/M = new turf_type
    // Stuff here

Apr 17 2014, 12:57 pm
That does explain something, but even with that change to while(1), it doesn't seem to get there.
Apr 17 2014, 1:01 pm
You definitely shouldn't be using while(1) there, unless you want to crash your game.

datum/controller/generator/map_generator/proc/SpawnMinerals()
    world.log << "HAI THERE"
    for(var/type_path in minerals)
        M = new type_path
        if(M)
            var/rarity = M.rarity
            var/turf/spawnloc = pick(turflist)
            var/spawn_amount = 5000
            while(spawn_amount)
                world.log << "While Loop"
                if(prob(rarity))
                    M.loc = spawnloc
                spawn_amount--
                sleep(1) // Prevent overload, if the spawn_amount wasn't so high you could get away without this.


Is working in my tests.
Apr 17 2014, 1:02 pm
In response to Laser50
Could the issue be with M = new(locate(1,1,1))? Why aren't you making M a new instance of M? or making a new mineral of M's type in that location? I think when your making M's new proc it's deleting M itself; this if(M) returns false.
Apr 17 2014, 1:03 pm
In response to Nadrew
Nadrew wrote:
You definitely shouldn't be using while(1) there, unless you want to crash your game.

> datum/controller/generator/map_generator/proc/SpawnMinerals()
>     world.log << "HAI THERE"
>     for(var/type_path in minerals)
>         M = new type_path
>         if(M)
>             var/rarity = M.rarity
>             var/turf/spawnloc = pick(turflist)
>             var/spawn_amount = 5000
>             while(spawn_amount)
>                 world.log << "While Loop"
>                 if(prob(rarity))
>                     M.loc = spawnloc
>                 spawn_amount--
>                 sleep(1) // Prevent overload, if the spawn_amount wasn't so high you could get away without this.
>

Is working in my tests.
Wouldn't this just move the same instance of M around 5000 times?
Apr 17 2014, 1:05 pm
From what I gathered in his code he's not trying to spawn 5000 mineral veins, but give 5,000 chances for that specific one to spawn.

If that's not what he intended, his code is outright wrong.
Apr 17 2014, 1:07 pm
Ahh I see, I was a bit confused with your spawn_amount variable. Sounds more like it should be named spawn_chance.
Apr 17 2014, 1:10 pm
The while(5000) was added as an attempt to get the object to spawn, even if it had overloaded, I would have known it worked. Original value was from a variable that was approx. 128.

Spawn chance would have been done later using probability (Which is the rarity value)

Since my previous attempts did not seem to like the rarity value of the turf I was attempting to spawn in. (thus, stopping the entire function), I tried to create the object first as a reference point, which would be used to gain the rarity value, and would then be deleted again. A rather inefficient solution, but it didn't work, any way.
Apr 17 2014, 1:19 pm
In response to Laser50
From what i can tell, you did exactly what you planned. Your only issues were your while loop not being formatted correctly or iterated, and you incorrectly initializing the reference mineral.
Apr 17 2014, 1:25 pm
Changed it all to the following, taking in the suggestions you guys posted (Hopefully I read it correctly, I'm pretty tired.)

datum/controller/generator/map_generator/proc/SpawnMinerals()
    for(var/spawn_type in minerals)
        var/turf/walls/mineral/M = new spawn_type
        if(M)
            var/rarity = M.rarity
            var/turf/spawnloc = pick(turflist)
            while(1)
                if(prob(rarity))
                    M.loc = spawnloc


But I can't change M's location because it's constant.
Apr 17 2014, 1:29 pm (Edited on Apr 17 2014, 1:37 pm)
In response to Laser50
while nothing is 5000 do something.

Nothing is 5000 would equate to false. This reads while 5000 is something, which equates to true.

It was incapable of finding M.rarity (null), even when I gave it the mineral type directly.

You can declare the rarity by creating an associative array that contains /path = rarity format. You can generate this in code by creating all types of turf and adding their path and rarity variables to the list so you would only need it once per server.

You can redefine pick on this list, so that it adds all the values together and does a weighted random selection.
Apr 17 2014, 1:50 pm
In response to Laser50
Laser50 wrote:
Changed it all to the following, taking in the suggestions you guys posted (Hopefully I read it correctly, I'm pretty tired.)

> datum/controller/generator/map_generator/proc/SpawnMinerals()
>   for(var/spawn_type in minerals)
>       var/turf/walls/mineral/M = new spawn_type
>       if(M)
>           var/rarity = M.rarity
>           var/turf/spawnloc = pick(turflist)
>           while(1)
>               if(prob(rarity))
>                   M.loc = spawnloc
>

But I can't change M's location because it's constant.


You probably shouldn't be using turfs for the minerals, but objects, turfs can't be moved at runtime, just created and destroyed.
Apr 17 2014, 2:08 pm
@Nadrew: You might find this an interesting read.
http://www.byond.com/forum/?post=1500081

It turns out, that updating turfs has seen major improvements, and in some cases, using objects to avoid the turf updates like we used to have to do to keep things streamlined may now, in fact, be slower than just using turfs for static entities.

As for your issue, Laser, the reason that a turf's loc is read only, is because atoms are not all movables. Movables are the only atoms that can move, and thus, turfs must be initialized at a specific location in order for this to work.

Second, your loop wouldn't actually work even if you used an object, because you can only assign one object one location. You'll notice that you are assigning the same thing to multiple locations, and that just won't work at all. You need to initialize an instance per location.

datum/controller/generator/map_generator/proc/SpawnMinerals()
    for(var/spawn_type in minerals)
        var/turf/spawnloc = pick(turflist)
        while(/*replace this with a more sane case*/)
            if(/*you need to store rarity some other way*/)
                new spawn_type(spawnloc)


I would recommend storing mineral rarity in an associative list:

var/mineral_rarity = list(/turf/walls/mineral/coal = 100,/turf/walls/mineral/iron = 75)


This way, you can grab the rarity by association to the type you are trying to initialize:

var/rarity = mineral_rarity[spawn_type]