Decorators vs the eventscripts way ..

Discuss API design here.
Omega_K2
Senior Member
Posts: 227
Joined: Sat Jul 07, 2012 3:05 am
Location: Europe
Contact:

Decorators vs the eventscripts way ..

Postby Omega_K2 » Sat Jul 07, 2012 1:19 pm

So I thought that in addons it might be better to use decorators instead of making some functions automatically work as it gives a bit more freedom. I also don't really like how es tries to register every functon as event (unless it starts with _)

Some ideas:

Syntax: Select all

@load
def myloadfunction():
print ("This will be loaded upon script load")

@load
def anotherloadfunction():
print("Hey, this will be loaded, too, but after the first function since it was declared later on :P")

@event
def player_hurt(ev):
# this registers this function as event listener; since no event specified, the function name (func.__name__) will be assumed to be the event
pass

@event("player_hurt")
def give_health_on_hurt(ev):
# This registers this function as event listener for player hurt
pass


Thoughts`?
User avatar
Ayuto
Project Leader
Posts: 2193
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Postby Ayuto » Sat Jul 07, 2012 2:40 pm

Nice idea! The same you could do for server, client and say commands and everything else you need to register on load and unregister on unload. In one of my scripts I use decorators for commands and SPE detours and it makes the addon more clearer :-)
your-name-here
Developer
Posts: 168
Joined: Sat Jul 07, 2012 1:58 am

Postby your-name-here » Sun Jul 08, 2012 2:35 am

Don't take this the wrong way as I am just trying to understand. What problem are decorators supposed to address?
User avatar
DanielB
Junior Member
Posts: 4
Joined: Sat Jul 07, 2012 2:33 am
Location: Australia
Contact:

Postby DanielB » Sun Jul 08, 2012 8:02 am

The current implementation in src/core/addons/es_addon.cpp works more-or-less exactly like ES2 does - checks the global namespace for variable with the same name as the event.
Decorators could provide a number of improvements on this model
  • Multiple handlers for an event
  • Able to use names for functions other than the event name
  • Ability to remove a handler
It also lends itself nicely to making "special case" events, such automatically passing an "attacker" and/or "victim" object by using a decorator along the lines of

Syntax: Select all

@event(players=['userid', 'attacker'])
def player_hurt(event_var, victim, attacker):
pass

@event('player_hurt', arguments=['userid', 'attacker', 'dmg_health'])
def some_func_name(victim, attacker, damage):
pass

Obviously this is a stylistic choice, and those particular examples are flawed.

If this were to be done I'd like to see each addon have to create an addon instance, similar to es.AddonInfo.
This would ensure each addon had some basic info (author, name, version, forum url, ...) and could provide other functionality.
so @event(...) would become @myAddon.event(...)
Omega_K2
Senior Member
Posts: 227
Joined: Sat Jul 07, 2012 3:05 am
Location: Europe
Contact:

Postby Omega_K2 » Sun Jul 08, 2012 9:38 am

Also something to note here is that decorators are implented, normal functions should still be implace; while decorators are truely useful in everyday scripting, functions for registering and unregistering event listeners/commands etc prove useful if you do the registering dynamically (for example, if you only listen to events if related config settings were enabled).

Dualism example:

Syntax: Select all

import sp.event
from sp.decorators import event, load

@load
def load():
global config
config = MyConfig()
if config['extrahealthonspawn']: # Dynamic registration
sp.event.Register(health, 'player_spawn')

@event
def player_hurt(ev):
pass # need this in any case

def health(ev):
pass #code here

def OnConfigChanged(config, oldconfig): # some more dynamic registration
if config['extrahealthonspawn'] and not oldconfig['extrahealthonspawn']:
sp.event.Register(health, 'player_spawn')
if oldconfig['extrahealthonspawn'] and not config['extrahealthonspawn']:
sp.event.Unregister(health)


Anyway, though I think it is generally a good idea to unregister events on unload automatically (I don't see why they should point to a non existing function).

DanielB wrote:If this were to be done I'd like to see each addon have to create an addon instance, similar to es.AddonInfo.
This would ensure each addon had some basic info (author, name, version, forum url, ...) and could provide other functionality.
so @event(...) would become @myAddon.event(...)


Yeah that sounds like a good idea as well.

your-name-here wrote:Don't take this the wrong way as I am just trying to understand. What problem are decorators supposed to address?


I think DanielB's post addresses this really well. Though in addition I don't think it is a good idea to just register any function that happens to have the same name as an event as cb. This functionality could be optional, but I don't think it should be default - in ES I think this was done due to legacy compability, but now we are not forced to do it that way.
If you happen to choose a name for a function that matches an event, you'll be registering an event without knowing it, similarily, you may not listen to events to want to listen to because they do not exist (though AFAIK you can't really check for event existence - but at least a warning could be shown). It also seems a bit of [unnecessary?] extra work to go though the script files and search for a matching event function everytime instead of going though a list of registered events/callbacks for a matching eventname.
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Postby satoon101 » Sun Jul 08, 2012 1:41 pm

DanielB wrote:
  • Multiple handlers for an event
  • Able to use names for functions other than the event name
  • Ability to remove a handler
I see all 3 of these as highly unnecessary, personally. If one of you wishes, you can make a python-side module that adds these decorators, and we can discuss whether or not we wish to implement it into the code base at a later time.

One idea that we have been throwing around is if a file named "events.py" is in the main directory of an addon, only functions within that particular file would be checked to see if the event needs registered, or possibly they will all be registered in case that addon adds its own new events. Since the events.py would be specifically designed for this functionality, it would only make sense to register all functions as events within that file. There will also be built-in functionality to register events manually.

Satoon
User avatar
DanielB
Junior Member
Posts: 4
Joined: Sat Jul 07, 2012 2:33 am
Location: Australia
Contact:

Postby DanielB » Sun Jul 08, 2012 3:42 pm

satoon101 wrote:I see all 3 of these as highly unnecessary, personally. If one of you wishes, you can make a python-side module that adds these decorators, and we can discuss whether or not we wish to implement it into the code base at a later time.

One idea that we have been throwing around is if a file named "events.py" is in the main directory of an addon, only functions within that particular file would be checked to see if the event needs registered, or possibly they will all be registered in case that addon adds its own new events. Since the events.py would be specifically designed for this functionality, it would only make sense to register all functions as events within that file. There will also be built-in functionality to register events manually.

Satoon


Not everything in the file would be an event - anything imported will also exist within the global namespace. I do think explicitly saying something is an event is better (and understand that having them in a specified file is explicit).
You are correct, though, in saying this could be implemented on top of whatever system is used - given there is a method to register/unregister.

Is there any documentation on any design choices that have already been made, like a sample addon?
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Postby satoon101 » Sun Jul 08, 2012 4:01 pm

DanielB wrote:Not everything in the file would be an event - anything imported will also exist within the global namespace.
We can easily check to see what module an object is actually created in, which would eliminate this issue. We currently do this within GunGame5.1, though for Source.Python, this would be built-in C++ side.

DanielB wrote:Is there any documentation on any design choices that have already been made, like a sample addon?
Unfortunately there is not any yet. We are still in the process of making design decisions, which is part of what this forum is for, and we welcome all suggestions. Again, what I wrote is "solely" my opinion. I am not opposed to having multiple ways to implement a "good" system for this. Though, I would also want that "multiple" to still be fairly limited to cut down on the necessary documentation and so that when new scripters come in, we have one or a very small few ways to steer them towards with different base functionality.

The current implementation was created to cut down on the number of registered event listeners, as ES2 registers "every" function within the script's main file for an event listener. The current way will only register functions in the script's main file when its name is a "known" event.

Satoon
your-name-here
Developer
Posts: 168
Joined: Sat Jul 07, 2012 1:58 am

Postby your-name-here » Sun Jul 08, 2012 10:59 pm

I should clarify that Source.Python does not "register" events per script. It parses all of the known event.res files and registers a listener for each event specified in them. When an event fires, it will see if a function with that name exists within the imported addon. If it does, it will call that function with an IGameEvent object. You guys should look at how Source.Python does that. It doesn't pass event variables as dictionaries. It passes the Source Engine's internal pointer as the parameter. Implementing decorators is going to be difficult on the C++ side. Is someone able to whip up a python-only version of this? I can implement some code that calls a central function from which you can then propagate out events to addons.
your-name-here
Developer
Posts: 168
Joined: Sat Jul 07, 2012 1:58 am

Postby your-name-here » Mon Jul 09, 2012 12:53 am

DanielB wrote:The current implementation in src/core/addons/es_addon.cpp works more-or-less exactly like ES2 does - checks the global namespace for variable with the same name as the event.
Decorators could provide a number of improvements on this model
  • Multiple handlers for an event
  • Able to use names for functions other than the event name
  • Ability to remove a handler
It also lends itself nicely to making "special case" events, such automatically passing an "attacker" and/or "victim" object by using a decorator along the lines of

Syntax: Select all

@event(players=['userid', 'attacker'])
def player_hurt(event_var, victim, attacker):
pass

@event('player_hurt', arguments=['userid', 'attacker', 'dmg_health'])
def some_func_name(victim, attacker, damage):
pass

Obviously this is a stylistic choice, and those particular examples are flawed.

If this were to be done I'd like to see each addon have to create an addon instance, similar to es.AddonInfo.
This would ensure each addon had some basic info (author, name, version, forum url, ...) and could provide other functionality.
so @event(...) would become @myAddon.event(...)


Again, don't take this the wrong way. I still am not seeing the value of this type of syntax. What does additional value does it provide? What problem does this solve?

Can you come up with a real-world example? I can't think of a case where you would want multiple load functions or multiple spawn functions. In fact, I feel that this would lead to confusing code because now you have multiple entry points in your addons that get called for a single event.
I am looking into the C++ code required to deal with decorators.

The problem that's being solved isn't clear to me. If someone can tell me what it is, I would be more than willing to listen and help you design a solution. However, I want to avoid unnecessary complexity at all costs. That complexity will affect both the C++ and Python sides of the house.
Omega_K2
Senior Member
Posts: 227
Joined: Sat Jul 07, 2012 3:05 am
Location: Europe
Contact:

Postby Omega_K2 » Mon Jul 09, 2012 6:38 am

Example implemenation in python, just to get an idea how it might work. I suppose this can be improved, but I won't have time for that until the weekend :p

http://omegak2.net/deco.zip

Same stuff on pastebin:
main.py
decorators.py
test.py
sp.py

Also note that this was made / tested with Python 2.7, so adjustments need to be made for 3.x
your-name-here
Developer
Posts: 168
Joined: Sat Jul 07, 2012 1:58 am

Postby your-name-here » Tue Jul 10, 2012 9:05 pm

This makes much more sense to me now. Thanks for the example Omega. I will modify SP to call your event manager code. I'll also commit your libs. Let's test this out :)
User avatar
Monday
Administrator
Posts: 98
Joined: Thu Jul 12, 2012 4:15 am

Postby Monday » Thu Jul 12, 2012 6:27 am

I find myself really on the fence about this.... If it can easily be done in c++, then it would be a great addition! If it proves to be a pain, I think this idea should be put on the back burner for now...

Return to “API Design”

Who is online

Users browsing this forum: No registered users and 22 guests