Added engine_import implementation. #277
Conversation
… loading engine/game specific files. Added "skippables" as an optional parameter to "engine_import" to allow skipping names explicitly. Support "<class>.<attribute>" format. Added "skip_privates" as an optional parameter to "engine_import" to allow skipping private names (e.g internal callbacks, etc.). Caller's docstring is now only overwritten if it was originally undocumented. Engine/game's "__all__" attribute is now merged with the caller's instead of being overwritten.
Ayuto
left a comment
There was a problem hiding this comment.
Looks good to me! Though, it might be worth mentioning what directory structure engine_import expects.
satoon101
left a comment
There was a problem hiding this comment.
Very nice! I did find a couple issues as noted in my review comments, but overall this is fantastic!
| obj = tuple(sorted(set(obj + getattr(caller, '__all__')))) | ||
| elif skip_privates and attr.startswith('_'): | ||
| continue | ||
| elif isclass(obj): |
There was a problem hiding this comment.
This line raises NotImplementedError on DOD:S and TF2 servers. I originally thought this was because we don't have any weapon data for those 2 games, but when I tested Garrysmod, it worked fine.
Traceback (most recent call last):
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\__init__.py", line 93, in load
setup_sp_command()
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\__init__.py", line 329, in setup_sp_command
from core.command import auth, docs, dump, plugin
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\core\command\__init__.py", line 20, in <module>
from commands.typed import TypedServerCommand
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\commands\typed.py", line 30, in <module>
from filters.players import parse_filter
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\filters\players.py", line 26, in <module>
from players.entity import Player
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\players\entity.py", line 1021, in <module>
engine_import()
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\core\__init__.py", line 390, in engine_import
elif isclass(obj):
File "E:\Servers\tf\tf\addons\source-python\Python3\inspect.py", line 78, in isclass
return isinstance(object, type)
File "E:\Servers\tf\tf\addons\source-python\packages\source-python\weapons\default.py", line 29, in __getattribute__
'No support for game "{0}"'.format(GAME_NAME))
NotImplementedError: No support for game "tf"If I wrap that isclass in a try/except, it works fine, but I'm not sure if there are any negative consequences of doing so.
There was a problem hiding this comment.
This happens because isclass is internally calling isinstance which attempts to get the __class__ attribute of the object but since NoWeaponManager is raising into __getattribute__, it fails. The same would occurs with:
from weapons.default import NoWeaponManager
mgr = NoWeaponManager()
mgr.__class__I don't believe we should wrap the exception because it is working as intended. That class is designed to raise in such scenario and I believe it should rather raise into __getattr__ instead so that it only fails when attempting to get attributes that are not contained in the __dict__ instance of the class itself.
| # Entities | ||
| from entities.entity import Entity | ||
| # Weapons | ||
| from _weapons._entity import WeaponMixin |
There was a problem hiding this comment.
When building the documentation, the _weapons module gives an ModuleNotFoundError:
E:\Servers\cstrike\cstrike\addons\source-python\docs\source-python\source\developing\modules\weapons.entity.rst:9: WARNING: autodoc: failed to import class 'WeaponMixin' from module '_weapons._entity'; the following exception was raised:
Traceback (most recent call last):
File "E:\Servers\cstrike\cstrike\addons\source-python\packages\site-packages\sphinx\ext\autodoc.py", line 518, in import_object
__import__(self.modname)
ModuleNotFoundError: No module named '_weapons'There was a problem hiding this comment.
EDIT: This was a warning caused by that module never being declared. Should be fixed there: 28ae74d
…, __getitem__ and __setitem__ rather than __getattribute__.
… class in the example of engine_import was redundant.
…et_wrapped used to emulate hierarchical calls.
Fixed some Entity/Player's extension methods for CS:GO.
…bind the wrapped method before returning it.
…sure that the original container type is retained after the merge has been performed.
Fixed docstrings being overwritten for injected properties.
This PR addresses the documentation issues that were discussed into the following thread: https://forums.sourcepython.com/viewtopic.php?p=10831#p10831