-
Notifications
You must be signed in to change notification settings - Fork 82
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
Handle unknown plane/helicopter types (mods) more gracefully/best effort #219
base: master
Are you sure you want to change the base?
Conversation
This became pretty relevant again when Black Shark 3 came out. It's a bit unfortunate/problematic that pydcs just breaks every time a new module comes out. Would be nice if such modules could just flow unchanged through the read/write scenario. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking so long. Overlooked this one but it came up again in #344.
I'm not really keen on treating things that are unknown so identically. The constructed type in this case will appear to be fully recognized and will have all the properties that a recognized aircraft would have, but all those properties would just be incorrect defaults.
If the behavior were controllable (for my use case, I want an exception raised, if there's something we don't recognize in the miz, something has gone wrong and we should stop processing) I wouldn't object, but my use case is very different from yours so @rp- is probably the one that should approve/deny (though from the other PR I can guess that there ought to be a test added that verifies that a miz that was deserialized in this manner still serializes correctly).
|
||
if imp_unit["type"] not in planes.plane_map: | ||
print("WARN:", "Unknown plane type", file=sys.stderr) | ||
plane_type = type(imp_unit["type"], (PlaneType,), {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty wary of this approach:
>>> with open(os.devnull, "w") as dev_null:
... pickle.dump(type("Bar", (Foo,), {})(), dev_null)
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
_pickle.PicklingError: Can't pickle <class '__main__.Bar'>: attribute lookup Bar on __main__ failed
I don't think it's important that a distinct type is created for each aircraft type, so PlaneType()
is probably plenty good.
This also does call type()
for every instance of the plane that isn't recognized. Experimentally that's not a problem in the repl (id(type("Foo")) == id(type("Foo"))
, but I can't find anything in the docs of type
that specify the behavior one way or the other so I'd prefer to not rely on it without a good reason.
I have currently not the time to fully rethink how unknown planes could be handled correctly. |
No description provided.