ID:139300
 
Code:
proc/genstair()
new /turf/Dungeon/StairUp(locate(pick(/turf/Dungeon/Floor)))
new /turf/Dungeon/StairDown(locate(pick(/turf/Dungeon/Floor)))


Problem description:

The stairs are generated adjacent to one another, every single time. It doesn't appear to be an issue of separating the two actions because this generates the same result, as does using a list.
You are only picking a single type: /turf/Dungeon/Floor. new /turf/Dungeon/StairUp(locate(pick(/turf/Dungeon/Floor))) is equivalent to new /turf/Dungeon/StairUp(locate(/turf/Dungeon/Floor)). Your turfs will always be adjacent to each other because, having replaced the original /turf/Dungeon/Floor, the new locate() will find a turf adjacent to it (from what you say, I'm assuming locate works deterministically, for example, from the bottom-up). To get the functionality you want, you need to construct a list of all dungeon turfs, and use pick() on that list. Read in the reference about how pick() works.
In response to Toadfish
Much obliged, I did misunderstand the use of the pick() proc and thought that I was adding all turfs of said type instead of just the one arg, which it would pick every time.
In response to Warpedlittleman
You can use pick() from a block() of turfs or from an /area.contents to achieve what you want.
I think what you really intended was pick(typesof(/turf/Dungeon/Floor)). However that's probably not going to give you the behavior you want anyway. The problem is, you'll be calling pick() to get a type, and then locate() will find the first instance of that type--it'll be as closed to the lower left of the map as possible.

What you probably want is to make a list of the acceptable turfs, and pick from those. One way to do that is to fill a list by looping through all /turf/Floor turfs in an area. Another option is to make a copy of area.contents, pick() from it, and then keep discarding choices that aren't /turf/Floor until you've run out of choices.

area/PickType(ttype)
// first try a raw pick
. = pick(contents)
if(istype(.,ttype)) return .
// now go to the trouble of copying the list
var/list/C = contents.Copy()
var/i
while(C.len)
i = rand(1, C.len)
. = C[i]
if(istype(.,ttype) return .
C[i] = C[C.len--]
return null


Note that the routine above will perform poorly if there are many turfs in the area, but only a few of the kind requested. If the ratio is much smaller, or at least potentially so, then I would recommend filling in your pick() list either by calling istype() on each turf, or using a hard-coded for() loop so you can use for(var/turf/Dungeon/Floor/T in area).

Lummox JR
In response to Lummox JR
I ended up doing this, and it gives the results I wanted:

<code> Genstairs() var/list/turfs = list() for(var/turf/Dungeon/Floor/T in block(locate(1,1,1),locate(world.xlimit,world.ylimit,1))) turfs += T var/turf/a=pick(turfs) turfs-=a new /turf/Dungeon/StairUp(a) new /turf/Dungeon/StairDown(pick(turfs)) del(turfs) ..()</Code>

Turns out when I was trying to use a list beforehand I was doing that wrong too. At some later point I'll probably go to the trouble of creating a persistent list for the generation of other objects as needed, but this works just fine until I decide how I want to handle that.

I'm curious as to how I could determine the relative efficiency of doing it this way, or using the method LummoxJR described, for different ratios of desired turf/not desired turf. Different methods I'm using for level generation are yielding results which are predictable enough that it might be useful to have both if the benefit is significant. Does anyone know how I could determine this, or at which point it becomes more efficient to use one method over the other?
In response to Warpedlittleman
In my opinion, assuming you'd be using an /area of size xlimit on ylimit, your code will most likely be somewhat less efficient, the reason being you're constructing two lists: a list of all turfs, and then a list of all turfs of a specific type inside the former, finally using pick() (correctly this time!) on that second list. I think this is true for both dense areas full of /turf/Dungeon/Floor, and areas sparse with them, although Lummox knows better than I do - and he claims your code would be more efficient in the latter case. You can analyse this more deeply, however, I'm not sure you and Lummox are justified in caring about the efficiency of this all: how many times do you plan to call Genstairs()? Once per dungeon floor? That doesn't justify optimization, does it?

By the way, I don't think you need to use del(turfs) there, DM would automatically clean up that list.
In response to Warpedlittleman
If you're gonna be picking multiple locations at a time like this, making a copy just once may be a good solution. Just doing two, it's kind of iffy which one will serve you better.

The method I outlined has a k in n+m chance of picking the turf type you want, where k is the number of that turf type, n is the total number of turfs, and m is the total number of movable atoms in the area. If it fails that first check it then has to make a copy of the contents list, and it will have the same k/(n+m) chance each time except that it will be reducing n+m whenever a match isn't found. The algorithm is approximately O(n/k) plus the overhead for copying the list.

The method you used always makes a list of the turf types you want, but has the advantage of only needing to make that list once. Overall it's probably faster than the method I outlined unless a raw pick() from contents would hit on both tries. If you have a floor-to-wall ratio of 0.5, that means your method has a 75% chance of outperforming the other one. The only significant advantage of doing it the other way is that it's fairly generic--it can be called from wherever.

Basically I think you found an optimal solution for this case, unless there's a lot more random turf picking going on in other routines in which case it'd make sense to save the list of available floor turfs for later. (My routine doesn't address this either.) Chances are you're not going to call this often enough for the list creation hit to ever be significant anyway. Where you really want to optimize is in routines that are called frequently.

Lummox JR
In response to Lummox JR
You're right, I analysed this wrong - I ignored the overhead from contents.Copy(), and the fact there will be movables in area.contents. Whoops. I still don't think there's a reason to care about the efficiency of this, though.