Skip to content

NetworkIDValidated listener and sperate "listener" module for listeners #1

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

Merged
merged 10 commits into from
Dec 28, 2013

Conversation

OmegaK2
Copy link
Contributor

@OmegaK2 OmegaK2 commented Sep 14, 2013

Look at files for details

- Fixed overriding functions with void as return type
- Changed DC_SIGCHAR_BOOL to 'b' (same for DynamicHooks)
@ghost ghost assigned satoon101 Sep 17, 2013
@satoon101
Copy link
Member

I like the idea and implementation. Are there any other listeners that would be worth adding?

The available ones are LevelInit, ServerActivate, LevelShutdown, ClientActive, ClientDisconnect, ClientPutInServer, ClientSettingsChanged, ClientConnect, and OnQueryCvarValueFinished. Some of those have an event equivalent, but some do not.

I am also still trying to figure out how to get the ellipsis working, but not having any luck.

If we do end up adding more listeners, I would prefer some of the declaration and implementation be consolidated.

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 21, 2013

Some of these already exist as event (ClientActive, ClientDisconnect, ClientConnect). I think adding the other should work.

As for the ellipsis (variadic template) - I've tested a bit and it seems VS2010 doesn't support it at all (and won't). VS2012 supports it but it seems you need a special compiler package for it to work, as it's not implemented in the VS211 compiler VS2012 uses by default:
http://msdn.microsoft.com/en-us/library/vstudio/hh567368.aspx
The VS212 compiler package:
http://www.microsoft.com/en-us/download/details.aspx?id=35515

I was thinking that instead of using VS Compiler, maybe trying to compile for windows with GCC works better, it's c++11 support seems almost complete:
http://gcc.gnu.org/projects/cxx0x.html

Anyway (either way), this also requires to recompile some of the libs linked against, like protobuf, but I've stopped experimenting further, and only done this so far with VS2012, not sure whether this C++11 support desireable in Source.Python, I think it is, but it seems going though a fair share of trouble getting it to work on windows in the first place, so I just went with a simpler solution of overloading the method intead for the time being.

@satoon101
Copy link
Member

Ok, I am fine with the overloads for now. Making such a big change just to implement the ellipsis probably isn't in our best interest at this time.

Go ahead and add in some of the other listeners, then I will retest and merge the changes.

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 21, 2013

Yeah, changing the compiler for windows is probably a sub-project on it's own. As for the listeners, sure I'll go ahead.

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 22, 2013

I've added them, but the are not implemented yet in sp_addon.cpp/sp_main.cpp
Do you think it's good to have empty bodies in sp_addon.cpp ? Essentially it's just a copy of the headers/functions and passing the call, not sure whether this is over the top, as one could just put it in sp_main directly.

So where do you want call in sp_addon.cp (sp_main->sp_addon->listener) or sp_main.cpp (sp_main->listener)?

As a follow-up on the varargs:

C also supports this, however I have no idea how to pass it on properly; I.e. you can define the function with an ellipsis and access the varargs, however the problem is passing it on as vars to CALL_PY_FUNC; plus, the type is unknown using the C varargs.

So my idea was to create a pylist (or dict) and pass it instead, but that still requires a bit of a hack, as there needs to be a way to determine the type of arg passed unless they are wrapped in an object containing the type; C++ is probably fast enough, but I still wasn't sure whether this is good approach, especially since more cpu-intensive events are used like ticks (up 128 times per second is lot) and the simpler approach probably works just as well (and there aren't that many overloads anyway! a lot of them use (edict_t *pEntity) or no args )

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 22, 2013

Allright - should be done.

All seem to fire just fine, except for on_query_cvar_value_finished. It just doesn't seem to fire at all.

As for client_connect, it seems that you can reject connection by setting the pointer to false, but I had trouble getting that to work in python (I suspect the var gets reallocated or something? ), so I just went with read-only copies.

Edit: Also test script: http://pastebin.com/nGNsJzT0

@satoon101
Copy link
Member

I definitely don't think a dictionary is the way to go. I definitely want arguments to be "arguments". I'm not too terribly worried about using an ellipsis for now, so just remove that part entirely. If we can find a way to implement it in the future, we will do so then. That type of internal change will not effect the usage.

I also noticed, previously, how OnQueryCvarValueFinished was not being called. Though, I only tested this on CS:GO.

Also, I wanted consolidation. 32 files, most of which do almost exactly the same thing, seems a bit much.

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 24, 2013

Well, obviously you could move all the listeners into a single file.

If the change is reverted, the files make more sense again as the indiviudual implementations that are not common will be found in them; I was also thinking that they could serve for future wraps, the client_connect thing comes to mind (in order to get connection rejection from python to work).

So before I do the change, what about the [other] repetitive code? Duplicates of listener calling body that only differs in the types of arguments passed.

As for the dicts, anothner reason that came to my mind is that if changes are made, existing scripts will not break, though of course people could always use listeners with mylister(_args, *_kwargs), much like events work.

@satoon101
Copy link
Member

It should be fine consolidated. If arguments are changed, those scripts could still be broken. Besides, that would be a fairly obvious fix, either way, and would be fixed very quickly. I would be fine with anything with a PLUGIN_RESULT return value to be implemented in another file (or really, all of them in one set of files).

@OmegaK2
Copy link
Contributor Author

OmegaK2 commented Sep 25, 2013

Changes made.

@satoon101 satoon101 merged commit c393226 into Source-Python-Dev-Team:master Dec 28, 2013
Ayuto pushed a commit that referenced this pull request May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants