Crashes upon map change.

All other Source.Python topics and issues.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Tue Mar 31, 2015 3:26 am

I made some more testing and it crashes there in the PlayerEntity class:

Syntax: Select all

# ../players/entity.py

@property
def userid(self):
"""Return the player's userid."""
return self.playerinfo.get_userid()


Replacing that with the following seems to work correctly:

Syntax: Select all

engine_server.get_player_userid(self.edict)


However, the objects are still freed and no longer valid.

EDIT: The only instance that seems to always been alive is the edict_t pointer. However, I agree caching the instances is definitely boosting the accessing speed and I'm wondering if hooking the CBaseEntity destructor and marking the object as no longer valid using a guard variable that then raise in __getattr__ could worth the effort. Seems to be the only direct way to know when the engine is freeding that specific entity pointer.
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Postby satoon101 » Wed Apr 01, 2015 2:25 pm

We could at least provide a refresh_attributes method that could either return True/False if successful/unsuccessful or raise an error which scripters would need to use try/except or suppress to get around.
Image
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Wed Apr 01, 2015 2:54 pm

The problem would still remains since the index itself could be given to a completly different entity at this point.
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Postby satoon101 » Wed Apr 01, 2015 3:02 pm

Well, couldn't we use the edict_t instance, since you said it would still be alive anyway, and attempt to get the index from that first? Then, if that is unsuccessful, we know that the specific entity instance is no longer valid. Or, does the edict_t instance get reused with a new index?
Image
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Wed Apr 01, 2015 3:14 pm

Actually, the index is simply the "slot" of the edict_t instance in the global array. While the index seems to be preserved for players from a map to another, it will be marked as freed for an entity and will be reused as soon as a new entity is created.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Sat Apr 04, 2015 3:36 pm

Based on Satoon's approach, I've added two helper classes to store player/entity instances that remove automatically instances deleted by the engine.

Simple example:

Syntax: Select all

from events import Event
from players.helpers import index_from_userid
from players.dictionary import PlayerDictionary

# Get a player dictionary...
players = PlayerDictionary()

@Event
def player_spawn(game_event):
# Get the player instance...
player = players[index_from_userid(game_event.get_int('userid'))]

# Print out something...
print(player.name, 'spawned!')


By default, player instances are stored by their index but if you want to store them by userid, you can easilly inherits of this class.

Syntax: Select all

from events import Event
from players.helpers import index_from_userid
from players.helpers import userid_from_index
from players.dictionary import PlayerDictionary

class Players(PlayerDictionary):
"""Player dictionary storing player instances by their userid."""
def __missing__(self, userid):
super(Players, self).__missing__(index_from_userid(userid))

def __delitem__(self, userid):
super(Players, self).__delitem__(index_from_userid(userid))

def _on_entity_deleted(index, ptr):
super(Players, self)._on_entity_deleted(userid_from_index(index))

# Get a player dictionary...
players = Players()

@Event
def player_spawn(game_event):
# Get the player instance...
player = players[game_event.get_int('userid')]

# Print out something...
print(player.name, 'spawned!')
Also, you can pass in your custom player/entity class.

Syntax: Select all

players = PlayerDictionary(MyCustomPlayerClass)


In conclusion, if you want to store player/entity instances in some other containers, this becomes your responsability to ensure the stored instances stay valid.
Predz
Senior Member
Posts: 158
Joined: Wed Aug 08, 2012 9:05 pm
Location: Bristol, United Kingdom

Postby Predz » Wed Apr 22, 2015 5:44 pm

After about 10 hours of trying to fix this found finally a way :) Well to be honest not really a fix just a work around but it seems to sort all my problems. Have posted below to show.

Give credit to Muerte for the idea :)

Syntax: Select all

##
##
##

from core.command import _core_command

from events import Event

from listeners import LevelShutdown

##
##
##

plugin = 'hw'

@Event
def server_spawn(game_event):
_core_command.load_plugin(plugin)

@LevelShutdown
def level_shutdown():
_core_command.unload_plugin(plugin)
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Wed Apr 22, 2015 8:55 pm

I'm confused, why are you deleting your post to repost it? :rolleyes:
User avatar
Mahi
Senior Member
Posts: 236
Joined: Wed Aug 29, 2012 8:39 pm
Location: Finland

Postby Mahi » Thu Apr 23, 2015 2:42 pm

Predz wrote:After about 10 hours of trying to fix this found finally a way :) Well to be honest not really a fix just a work around but it seems to sort all my problems. Have posted below to show.

Give credit to Muerte for the idea :)

Syntax: Select all

##
##
##

from core.command import _core_command

from events import Event

from listeners import LevelShutdown

##
##
##

plugin = 'hw'

@Event
def server_spawn(game_event):
_core_command.load_plugin(plugin)

@LevelShutdown
def level_shutdown():
_core_command.unload_plugin(plugin)

I'm actually working on something better, but meanwhile this works great! :)
Predz
Senior Member
Posts: 158
Joined: Wed Aug 08, 2012 9:05 pm
Location: Bristol, United Kingdom

Postby Predz » Wed Apr 29, 2015 7:03 pm

Okay, so another problem to help me debug :)

Essentially, Mahi's fix works sort of. The map now changes and anyone not on the server before the map change can play. However anyone on the server before the map change will crash their client when they try to join a team that isn't spectating. Any ideas?

The server still runs and bots are running around the map fine when I go into spectator, however joining a team crashes the client instantly.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Wed Apr 29, 2015 9:45 pm

You are most likely crashing at this line. However, you guys are making it hard to debug using unecessary ways to achieve things. For example, this class seems more than useless to me. If you want to cache attributes: store the objects. I believe you are crashing cause this results to the same thing: you are caching and retrieving attributes that needs to be refreshed (most likely since you execute Skill.execute_method whichs inheriths of Entity and then call player_spawn in the hero's class). Anyways, I think it would be better to store your data outside of your Player/Entity class since you are not controlling the lifetime of those: you rely on the engine. To be honnest, I would rather suggest you to use an external dictionnary that stores custom data by player's userid (since this is the only value you know won't change as long as players are connected). And then, when you initialize your custom Player class, retrieve that data or create it if it doesn't exists. I don't have time to write an example currently but this is an easy concept and probably the safest.
User avatar
Mahi
Senior Member
Posts: 236
Joined: Wed Aug 29, 2012 8:39 pm
Location: Finland

Postby Mahi » Thu Apr 30, 2015 11:02 am

L'In20Cible wrote:I believe you are crashing cause this results to the same thing: you are caching and retrieving attributes that needs to be refreshed (most likely since you execute Skill.execute_method whichs inheriths of Entity and then call player_spawn in the hero's class).

While I do agree with you that we would be better off having our data in a completely separate dictionary, it would complicate creating heroes (and skills) and the whole idea of this mod was to make that as easy as possible. In fact I started out by creating an example hero using an ideal format, and then coded my mod around that format.

I also disagree that we're not caching and retrieving any attributes that need to be refreshed. I'm only using CachedAttr for Hero-Wars' custom attributes, such as player's gold, current hero and heroes list. None of those need to be refreshed when a map changes, and I'm storing and retrieving them using player's steamid, which surely doesn't change.

Just wanted to point that out, since the main goal of the mod is to make it easy for hero makers to add new heroes, so I will have to bend in certain places. Which is why I'd prefer the current way if I can simply fix the crashing.

Edit: I am unable to reproduce the crash, could be something with Linux servers...
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Fri May 01, 2015 6:12 am

Mahi wrote:While I do agree with you that we would be better off having our data in a completely separate dictionary, it would complicate creating heroes (and skills) and the whole idea of this mod was to make that as easy as possible. In fact I started out by creating an example hero using an ideal format, and then coded my mod around that format.
I'm not fully sure how it would complicate heroes creation. The heroes themselves are inheriting of a class who can easilly manage the custom data internally.

Mahi wrote:I also disagree that we're not caching and retrieving any attributes that need to be refreshed. I'm only using CachedAttr for Hero-Wars' custom attributes, such as player's gold, current hero and heroes list. None of those need to be refreshed when a map changes, and I'm storing and retrieving them using player's steamid, which surely doesn't change.
Still, you are storing objects by their id (memory address) which is most likely going to leaks or get values of garbage collected objects from a new one which reused the pointer. This is dangerous since the id itself is not a reference but only an integer independant of the object itself (which means you will most likely store freed pointers and grow in size till you hit a MemoryError).
User avatar
Mahi
Senior Member
Posts: 236
Joined: Wed Aug 29, 2012 8:39 pm
Location: Finland

Postby Mahi » Fri May 01, 2015 3:34 pm

L'In20Cible wrote:I'm not fully sure how it would complicate heroes creation. The heroes themselves are inheriting of a class who can easilly manage the custom data internally.
Say you wanted to create a skill that gives gold to a player. It would be much harder, since now you can just do player.gold += 15. But I'll think about it, maybe I can sacrifice the feature since it's not really too often that you need to access the Hero-Wars sided data in a skill :)
L'In20Cible wrote:Still, you are storing objects by their id (memory address) which is most likely going to leaks or get values of garbage collected objects from a new one which reused the pointer. This is dangerous since the id itself is not a reference but only an integer independant of the object itself (which means you will most likely store freed pointers and grow in size till you hit a MemoryError).
Ah, true! Hadn't even thought about the id problem, maybe I'll just do what you suggested, maybe come up with something different. But thanks for the help :)
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Postby L'In20Cible » Fri May 01, 2015 6:44 pm

Mahi wrote:Say you wanted to create a skill that gives gold to a player. It would be much harder, since now you can just do player.gold += 15. But I'll think about it, maybe I can sacrifice the feature since it's not really too often that you need to access the Hero-Wars sided data in a skill :)
Oh but like I said, your base class can manage the custom data internally. :P

Syntax: Select all

some_dict = dict(gold=0)

class SomeBaseClass(object):
def __getattr__(self, attr):
return some_dict[attr]

def __setattr__(self, attr, value):
some_dict[attr] = value

class SomeClass(SomeBaseClass):
pass

obj = SomeClass()
print(obj.gold) # >> 0
obj.gold += 15
print(obj.gold) # >> 15
User avatar
Mahi
Senior Member
Posts: 236
Joined: Wed Aug 29, 2012 8:39 pm
Location: Finland

Postby Mahi » Sat May 02, 2015 9:56 am

L'In20Cible wrote:Oh but like I said, your base class can manage the custom data internally. :P

Syntax: Select all

some_dict = dict(gold=0)

class SomeBaseClass(object):
def __getattr__(self, attr):
return some_dict[attr]

def __setattr__(self, attr, value):
some_dict[attr] = value

class SomeClass(SomeBaseClass):
pass

obj = SomeClass()
print(obj.gold) # >> 0
obj.gold += 15
print(obj.gold) # >> 15
Oh, right!

What do you think, would it be better to do it like you're suggesting (inherit HW's Player from SP's PlayerEntity and use an external dict with __setattr__ and __getattr__), or have something like this:

Syntax: Select all

class Player(object):
def __init__(self, userid, gold):
self.userid = userid
self.gold = gold

def get_entity(self):
return PlayerEntity(index_from_userid(self.userid))

def __getattr__(self, attr):
return getattr(self.get_entity(), attr)

def __setattr__(self, attr, value):
setattr(self.get_entity(), attr, value)
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Postby satoon101 » Mon May 04, 2015 1:28 am

For players, I would use something more like:

Syntax: Select all

from events import Event
from listeners import ServerShutdown
from players.entity import PlayerEntity
from players.helpers import index_from_userid


class PlayerDictionary(dict):
def __missing__(self, userid):

# PlayerEntity here could also be a class that inherits from PlayerEntity
value = self[userid] = PlayerEntity(index_from_userid(userid))
return value

def __delitem__(self, userid):
"""This method should also be used to perform any functionality on the removal of a player.

For instance, you should stop player specific delays/repeats that you might have stored with the PlayerEntity instance.
"""
if userid in self:
super(PlayerDictionary, self).__delitem__(userid)

def clear(self):
"""Call self.__delitem__ for each userid in the dictionary."""
for userid in self:
del self[userid]

player_dictionary = PlayerDictionary()


@Event
def player_disconnect(game_event):
del player_dictionary[game_event.get_int('userid')]


@LevelShutdown
def level_shutdown():
"""Clear the dictionary at the end of the map when players are no longer 'on' the server."""
player_dictionary.clear()
Image

Return to “General Discussion”

Who is online

Users browsing this forum: No registered users and 39 guests