pfPython Cleanups #601
pfPython Cleanups #601
Conversation
|
This was really way too much. I'll wait for everyone to finish looking before pushing any fixes... |
|
I rebased this onto master and fixed the issues previously pointed out to facilitate review. |
| IBuildTupleArgs<sizeof...(args)>(tuple.Get(), std::forward<Args>(args)...); | ||
|
|
||
| plProfile_BeginTiming(PythonUpdate); | ||
| pyObjectRef retVal = PyObject_CallObject(callable, tuple.Get()); |
There was a problem hiding this comment.
The old code called PyObject_CallMethod(callable, fPyFunctionNames[methodId], ...), which actually invokes callable.name(*args) whereas this runs callable(*args)... Am I missing something?
There was a problem hiding this comment.
I reworked the way the callables are stored internally. Previously, it stored an array of pointers mapping method ID to class instances that was resolved at AddTarget() time. There should really only be one class instance in plPythonFileMod, so this was redundant. I changed the array to map method IDs to the actual method callables themselves.
This removes the ability to do fancy metaprogramming hacks like changing the OnNotify method, but that was flakey at best before considering if the callable was not a member of the class instance at AddTarget() time, said method would never be called, ever.
| // target's key to the pySceneObject. | ||
| if (fInstance) { | ||
| pyObjectRef pSceneObject = PyObject_GetAttrString(fInstance, "sceneobject"); | ||
| pySceneObject::ConvertFrom(pSceneObject.Get())->addObjKey(sobj->GetKey()); |
There was a problem hiding this comment.
This removes the opportunity for Python to overload the addKey method...
There was a problem hiding this comment.
The original intent of this PR was to rewrite glue.py in C++. This is happening deep inside the glue, I think, so I'm wondering how much we care?
|
Updated to address issues from review. Please let me know if there any other concerns! |
This does a number of things that make plPythonFileMod much easier to play with. Namely: - An RAII helper class `pyObjectRef` has been added that acts much like `std::unique_ptr` in that it steals a `PyObject*` and decrefs it on destruct. This helps eliminate lots of tedious code. - Removed a lot of boilerplate commentary that was copied/pasted around from the beginning of time. - Removed the specialization on internal clients killing erroring python methods. This did not make sense as we actually would rather error *more* in an internal client since that's who is actually testing the code.
This adds a helper for determining if a script has a method and the message matches.
This changes the way script methods are called. Instead of the verbose calling we did previously, we now have a variadic template powered calling function for PFM methods. I had to change fPyFunctionInstances to actually hold a pointer to the methods instead of a pointer to the PFM instance... Who thought it was a good idea to have a 30+ member array that points to either fInstance or null???
This should clean up the remaining style, const, and temporary copying turds in plPythonFileMod.
Among the stylistic fixes there are also fixes for PyTuple initialization, a PyObject* leak, and some various modernization tweaks.
This issue was exposed by H-uru#601. The Relto bahro pole smokers would intentionally use a negative value as the timer callback ID. While this is strictly speaking not kosher according to the `PtAtTimeCallback` docstring, the implementation gladly accepted signed integers. Additionally, throughout the code, -1 was used as a magic value. Therefore, we fix everyone to use signed integers for timer callback IDs. plPythonFileMod is now correctly passing signed values to the PFM's `OnTimer` method. Fixes the observed Relto traceback: (05/17 16:02:38) cPythBahroPoles - Traceback (most recent call last): (05/17 16:02:38) File ".\python\psnlBahroPoles.py", line 679, in OnTimer (05/17 16:02:38) self.RunState(self.PoleIDMap[ageID], state, 0) (05/17 16:02:38) KeyError: 429496729
This issue was exposed by H-uru#601. The Relto bahro pole smokers would intentionally use a negative value as the timer callback ID. While this is strictly speaking not kosher according to the `PtAtTimeCallback` docstring, the implementation gladly accepted signed integers. Additionally, throughout the code, -1 was used as a magic value. Therefore, we fix everyone to use signed integers for timer callback IDs. plPythonFileMod is now correctly passing signed values to the PFM's `OnTimer` method. Fixes the observed Relto traceback: (05/17 16:02:38) cPythBahroPoles - Traceback (most recent call last): (05/17 16:02:38) File ".\python\psnlBahroPoles.py", line 679, in OnTimer (05/17 16:02:38) self.RunState(self.PoleIDMap[ageID], state, 0) (05/17 16:02:38) KeyError: 429496729
This issue was exposed by H-uru#601. The Relto bahro pole smokers would intentionally use a negative value as the timer callback ID. While this is strictly speaking not kosher according to the `PtAtTimeCallback` docstring, the implementation gladly accepted signed integers. Additionally, throughout the code, -1 was used as a magic value. Therefore, we fix everyone to use signed integers for timer callback IDs. plPythonFileMod is now correctly passing signed values to the PFM's `OnTimer` method. Fixes the observed Relto traceback: (05/17 16:02:38) cPythBahroPoles - Traceback (most recent call last): (05/17 16:02:38) File ".\python\psnlBahroPoles.py", line 679, in OnTimer (05/17 16:02:38) self.RunState(self.PoleIDMap[ageID], state, 0) (05/17 16:02:38) KeyError: 429496729
This was originally intended to be a set of changes to rewrite the glue.py file in C++. However, things got a little out of hand in the associated cleanups, so here they are instead. They include:
This depends on H-uru/moul-scripts#110