Page 2 of 2

Posted: Tue Mar 31, 2015 3:26 am
by L'In20Cible
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.

Posted: Wed Apr 01, 2015 2:25 pm
by satoon101
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.

Posted: Wed Apr 01, 2015 2:54 pm
by L'In20Cible
The problem would still remains since the index itself could be given to a completly different entity at this point.

Posted: Wed Apr 01, 2015 3:02 pm
by satoon101
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?

Posted: Wed Apr 01, 2015 3:14 pm
by L'In20Cible
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.

Posted: Sat Apr 04, 2015 3:36 pm
by L'In20Cible
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.

Posted: Wed Apr 22, 2015 5:44 pm
by Predz
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)

Posted: Wed Apr 22, 2015 8:55 pm
by L'In20Cible
I'm confused, why are you deleting your post to repost it? :rolleyes:

Posted: Thu Apr 23, 2015 2:42 pm
by Mahi
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! :)

Posted: Wed Apr 29, 2015 7:03 pm
by Predz
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.

Posted: Wed Apr 29, 2015 9:45 pm
by L'In20Cible
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.

Posted: Thu Apr 30, 2015 11:02 am
by Mahi
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...

Posted: Fri May 01, 2015 6:12 am
by L'In20Cible
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).

Posted: Fri May 01, 2015 3:34 pm
by Mahi
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 :)

Posted: Fri May 01, 2015 6:44 pm
by L'In20Cible
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

Posted: Sat May 02, 2015 9:56 am
by Mahi
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)

Posted: Mon May 04, 2015 1:28 am
by satoon101
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()