Better late than never - Anon
first make it work, then make it right, and, finally, make it fast. -
Kernighan & Johnson
Rules of optimization:
(2) (for experts only) Don't yet.
(3) Profile before optimizing
First make it work, then make it right
There's a bug. If
$game_map#passable? always returns true, the
coordinates returned by this code are in this pattern:
Based on your description, you are (or were) looking for this pattern:
The trouble is here:
radius.include?([a + 1, a]) &&
radius.include?([a - 1, a]) &&
radius.include?([a, a + 1]) &&
radius.include?([a, a - 1])
One fix is to replace those lines with:
radius.reject do |x, y|
x.between?(@x - 1, @x + 1) && y.between?(@y - 1, @y + 1)
and finally, make it fast
With the above fix, the code works, so let's benchmark it. 10,000
iterations of it take 0.600 seconds on my box.
We can do better. Instead of adding coordinates just to remove them
later, how about not adding them in the first place?
r = 4
radius = 
for x in @x-r..@x+r
for y in @y-r..@y+r
next if x.between?(@x - 1, @x + 1) && y.between?(@y - 1, @y + 1)
radius.push([x, y]) if $game_map.passable?(x, y)
This is simpler, but is it faster? Just a bit. 10,000 iterations now
take 0.500 seconds instead of 0.600. More important, though, it's
There doesn't seem to be much room for optimization here, which leads
to these questions:
- Is the program fast enough?
- If not, which part of the program is the bottleneck?
You can't answer those questions without profiling. That's why the
first code you write should be clear and correct. Then, only if it isn't
fast enough, profile to learn where it's too slow, and optimize that
part. Repeat until it's fast enough, then stop.
The reason to optimize later rather than earlier is because
optimization has a cost. It increases the complexity of the code,
often leading to code that's harder to read and maintain. It's no
good to pay that cost unless you're getting actual benefit from it.
An OOP issue
This code should probably be in GameMap:
def passable_coords_near(x, y)
r = 4
radius = 
for x in (x - r)..(x + r)
for y in (y - r)..(y + r)
next if x.between?(x - 1, x + 1) && y.between?(y - 1, y + 1)
radius.push([x, y]) if passable?(x, y)