Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applies a temporary fix over player_list having a null until the proper solution is found #28751

Closed
wants to merge 3 commits into from

Conversation

lzimann
Copy link
Contributor

@lzimann lzimann commented Jun 24, 2017

So basically something is not being removed from the GLOB.player_list properly. This doesn't seem to happen as often(I believe pAIs were causing it aswell) but they still do.

I'm well aware this is not the solution to the problem, but a temporary bandaid is better than bleeding runtiming to death.

@lzimann lzimann added the Fix Rewrites a bug so it appears in different circumstances label Jun 24, 2017
@lzimann lzimann changed the title Applies a temporary over player_list having a null until the proper solution is found Applies a temporary fix over player_list having a null until the proper solution is found Jun 24, 2017
@MrStonedOne
Copy link
Member

MrStonedOne commented Jun 24, 2017

This is always related to somebody getting kicked back to lobby. it tends to be a runtime in destroy preventing ..() from getting called.

Edit, could also be logout, i can't remember what assigns to player list

@lzimann
Copy link
Contributor Author

lzimann commented Jun 24, 2017

I'll look over that aswell.

@ChangelingRain
Copy link
Contributor

could you log the type of the null somehow, maybe by making it an assoc list via player_list[src] = type and outputting the type when you discover it?

@lzimann
Copy link
Contributor Author

lzimann commented Jun 24, 2017

Could probably be done and way more helpful than a stack trace.

@lzimann
Copy link
Contributor Author

lzimann commented Jun 24, 2017

Also @MrStonedOne
Login will assign the player to the list.
Logout will remove.
So in this case, a Logout is not being properly called, the mob is destroyed(left as null) but never removed from the list.

@lzimann
Copy link
Contributor Author

lzimann commented Jun 24, 2017

I am not totally sure if it will keep the type if it becomes a null, but better than nothing.

@@ -225,8 +225,8 @@ GLOBAL_LIST_INIT(department_radio_keys, list(
for(var/_M in GLOB.player_list)
var/mob/M = _M
if(!M)
stack_trace("Null detected in player_list. Type: [GLOB.player_list[M]]")
listclearnulls(GLOB.player_list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably remove this in case there's multiple nulls, which could be important info.

@@ -224,6 +224,10 @@ GLOBAL_LIST_INIT(department_radio_keys, list(
var/list/the_dead = list()
for(var/_M in GLOB.player_list)
var/mob/M = _M
if(!M)
stack_trace("Null detected in player_list. Type: [GLOB.player_list[M]]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a flaw here.

@Cyberboss
Copy link
Member

Cyberboss commented Jun 25, 2017

If the problem is Destroy runtiming, maybe instead, add this line after the Destroy call in qdel?

if(<no_qdel_hint> && istype(D, /mob))
    message_admins("It seems mob/Destroy() crashed please check the runtime logs")

@lzimann
Copy link
Contributor Author

lzimann commented Jun 25, 2017

Feels kinda awful to add that check to qdel though.

@Cyberboss
Copy link
Member

Cyberboss commented Jun 25, 2017 via email

@lzimann
Copy link
Contributor Author

lzimann commented Jun 26, 2017

Eh, this would require proper tracking, which then would require a bunch of changes to player_list.

@lzimann lzimann closed this Jun 26, 2017
@lzimann lzimann deleted the bandaid branch June 26, 2017 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Rewrites a bug so it appears in different circumstances
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants