ID:157027
 
A few days ago, while working on my game i accidentally created a condition wherby normal usage of standard verbs caused a rather big (Not Responding) issue with Dream Seeker.

It actually took me a good 30 mins to realise that what id done. Which was create a lot of /mob bots on the map (to test something) and forget to removed them. But also that the method of doing something in the game currently has the potential to do this if there are a lot of players online.

In my game at the moment, when a player begins a Duel, it adds them to a list connected to them called 'SeeDuel'. It's straight forward enough it just does += usr. Then when they connect to another person (opponent) it adds them to the list.

Now when they talk, or something is output(ed) relating to the Duel, it sweeps the world like so...
for(var/mob/M in world)
if(usr in M.SeeDuel) // If the person who said/did something is in M's SeeDuel List...
M << "Something"


The issue that i caused was that by having a large number of mobs on an area of the map (it was probably something extreme here like 300-500) sweeping through them all to check if they should see what I was saying was eating so much CPU time that Dream Seeker took about 10-15 seconds of inactivity to finally do the M << "Something".

I have to wonder what would have happened if this was a server online...with lag etc.

The SeeDuel is also used for people viewing the Duel. So if they start watching one in progress, it add's that usrs SeeDuel to theirs. So they now see everything that player can see in outputs (plus it adds themselves of course).

I'm trying to think of a better way to do this, since it seems like it would be better not to loop through the world. I have had a few ideas but im sure that other people will probably have a better ones (since im not exactly the most code savvy, and not above asking for help when i need it).

I did consider instead of in world, doing in view(). But then there is a method for a user to "view" a Duel without being near them. This is especially important for a method of Dueling where the players are not stationary... so not really a good option.

Any input into this would be helpful. Perhaps looping through the world isnt that bad, in which case i dont need to change anything. It's just the incident caused by a large number of mobs made me start thinking.... If this is going on with lots of players on a server more or less constantly... would this be the source of lag.
going through every mob in the world is a bad way, you should use client...
for(var/client/C)
if(src in C.mob.SeeDuel)
C<<"something"
You should be using the tree to define different types of mobs.

mob/player

mob/enemy
goblin
ogre
etc


Then you can just loop through the actual mobs you want to loop through.

proc/test()
for(var/mob/player/p in world)
world << p


There's no reason at all that you shouldn't be using the tree and specifying your own types of an atom. This way each atom only has variables, procedures, verbs, etc that they need.

http://www.byond.com/members/ DreamMakers?command=view_post&post=39514

http://www.byond.com/members/ DreamMakers?command=view_post&post=36663

[Edit]
Also, if you are keeping a list of players connected to the mob why don't you just loop through that?

mob/player/proc/test()
for(var/mob/player/p in src.seeduel)
world << p

Just output to the SeeDuel list, assuming it's reciprocated (X in Y.SeeDuel iff Y in X.SeeDuel).
In response to Ulterior Motives (#2)
Creating new types does not improve the speed of loops. A for() loop can filter out turfs from mobs from objs from areas and so on because they all have different blocks of memory they reside in. However, a /mob/blah is still in the same block as a /mob/whatever, so to find all /mob/whatevers it has to loop through every /mob/blah as well. You might see some improvement because instead of an "in" check it's only performing an istype() check, but it doesn't really get at the core of the issue.
In response to Garthor (#3)
Yes, do you mean just go right to the mob in the list?

for(var/mob/M in usr.SeeDuel) M << "Hello!"


This is what id kinda decided was the better idea. However that causes a few issues with people who wish to watch them and so view their outputs.

While a player and their opponents would always be in the SeeDuel list, when you start watching someone thats another story. It would have to add someone who watchs you to everyone in your list SeeDuel, and then you to theirs. But then when you stopped, take you out again. I concluded this could probably cause bugs and glitchs if someone is clicking watch at the same time as someone else since it would be copying lists that are being changed by someone else etc.
In response to Garthor (#4)
Wow, I thought this would only loop through mobs of the type players. This is good to know.
In response to Masschaos100 (#1)
That's a good point too, wonder if it would be better.
In response to EternalDuelistSoul (#5)
Two things happening "simultaneously" will NEVER cause an error so long as you don't have any sleep() or spawn() statements to allow one to happen in the middle of another.

You don't need to loop through anything, here. Just usr.SeeDuel << "Hello!", in the same exact way you'd do view() << "Hello!".

As for whatever else you said, it sounds like you just have things organized in a terrible way, and you should have a single datum that handles the duel or whatever and have it relay messages as appropriate.
In response to Garthor (#8)
Garthor wrote:
As for whatever else you said, it sounds like you just have things organized in a terrible way, and you should have a single datum that handles the duel or whatever and have it relay messages as appropriate.

He probably doesn't understand what that means, and he also didn't probably look at the links that Ulterior Motives gave him that explained the meaning. So for Eternal Duelist's sake I will give him the link again

http://www.byond.com/members/ DreamMakers?command=view_post&post=36663

Read this my friend. If it doesn't make sense, read it again. It is important.