ID:1372951
 
(See the best response by Ss4toby.)

Problem description:

Hey guys. I've been working on a system which manages resources on the map; when it's started it picks out available turfs from a specific area and spawns resources there.

Problem is, what I've written to do that isn't very good. For some reason I can't think of a proper way to efficiently pick a random amount of things out of a list. Anyone have any ideas? This is what I'm trying to do.

        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var turf/cturf
var runcount = 0
while(runcount < Area.contents.len)
cturf = pick(Area.contents)
if(!cturf.PMade)
if(!available.Find(cturf)) // we don't want duplicates
available += cturf

if(available.len >= counter)
break
runcount++
oresOn(available)


Here's a quick rewrite because I don't understand the question:
...
var stuff[] = Area.contents.Copy()
while(stuff.len && available.len < counter && runcount++ < stuff.len)
cturf = pick(stuff)
stuff -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)
In response to Kaiochao
That's definitely a step in the right direction, although I don't think it was quite the solution. I adapted it into this:

        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var list/looping = Area.contents.Copy()
var turf/cturf
var maxcount = looping.len
var runcount = 0
while(looping.len && (available.len < counter) && (runcount++ < maxcount))
cturf = pick(looping)
looping -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)


Since we have to stop the loop at some point, my goal with runcount is to only run the loop for however many turfs were in the area.

If the area contained 500 turfs, then it doesn't make much sense to run the loop more than 500 times.

The way you've done it makes it so that it's possible to check for duplicates and limit it via the amount of times it was ran, which is cool.

The game still locks up when I use this though..

Edit: The check for looping.len should do that on it's own, hm. I'll make some changes.

        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var list/looping = Area.contents.Copy()
var turf/cturf
while(looping.len && (available.len < counter))
cturf = pick(looping)
looping -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)
To eliminate lock-up, use set background = 1

        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var list/looping = Area.contents.Copy()
var turf/cturf
while(looping.len && (available.len < counter))
set background = 1
cturf = pick(looping)
looping -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)


Honestly though, the script doesn't look like it should be locking up. Could be whatever is in oresOn()?

Also, a side note. Although I find the method of removing from the list more appealing, a good way to avoid adding dupes to a list is list |= value
In response to Ss4toby
Yeah, I tested that. When I run this, oresOn() isn't included. Just the loop locks up. For some reason the loop isn't ending.
Any runtime errors? Based on what I see, the loop shouldn't be stuck in inf.
Best response
Also! Forgot to mention this. Area.contents doesn't just return turfs, it returns everything in the area. So, you may want to utilize a for(var/turf/T in Area.contents) instead of a loop, and then just break. Of course, this would require re-designing the entire system >.<..

I suppose something like this might help, but is not idea:
        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var list/looping
var turf/cturf
for(var/turf/t in Area.contents)
set background = 1
looping+=t
while(looping.len && (available.len < counter))
set background = 1
cturf = pick(looping)
looping -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)


Honestly, the best method I can think of is to forget using area.contents all together and use block..

        populate()
var counter = Tin + Copper + Iron + Silver + Gold
var list/available = list()
var list/looping=block(locate(1,1,1),locate(world.maxx,world.maxy,1)
var turf/cturf
while(looping.len && (available.len < counter))
set background = 1
cturf = pick(looping)
looping -= cturf
if(!cturf.PMade)
available += cturf
oresOn(available)


The given example will return all turfs on the entire z-level.
Aha, that was the problem. I wasn't aware that areas contained turfs AND the turfs' contents. Changing it to this works out perfectly.

The latter example MAY be better, I'm not sure, but for my purposes no. This proc is designed under a special manager datum for an area's resources. Not all my areas contain a square block of turfs, so it has to be area specific :)

This is resolved.
Thanks both Kaiochao and Ss4toby