Skip to content
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

Medical Dependency Tree Issue #8389

Closed
kymckay opened this issue Aug 23, 2021 · 4 comments · Fixed by #8390
Closed

Medical Dependency Tree Issue #8389

kymckay opened this issue Aug 23, 2021 · 4 comments · Fixed by #8390

Comments

@kymckay
Copy link
Member

kymckay commented Aug 23, 2021

Just reviewing some of the items from the draft changelog. I don't believe this PR should have been merged as is, medical engine cannot depend on a setting from medical damage due to the dependency tree of those components.

Either medical engine must now depend on medical damage (creating a loop in the dependency graph) or this setting should be moved to medical engine where it takes effect.

Fwiw, there was some reason this setting was removed in the rewrite, but I forget what it was. I think it may be because there's no way to consistently detect all vehicle crashes (as reflected in the code comments).

Originally posted by @kymckay in #8149 (comment)

@kymckay kymckay added this to the 3.14.0 milestone Aug 23, 2021
@kymckay
Copy link
Member Author

kymckay commented Aug 23, 2021

Marked critical since it introduces an implicit dependency and should be fixed before next release, in reality it's unlikely anyone would ever load one without the other.

@jonpas
Copy link
Member

jonpas commented Aug 23, 2021

this setting should be moved to medical engine where it takes effect.

This.

@LinkIsGrim
Copy link
Contributor

LinkIsGrim commented Aug 23, 2021

Would moving it to main medical component work instead? medical engine has no settings file currently.
Nevermind, that is still dependent on medical engine.

PabstMirror added a commit that referenced this issue Aug 23, 2021
* fix implicit dependency between medical damage and medical engine

* move stringtable

* valve pls fix

Co-authored-by: PabstMirror <pabstmirror@gmail.com>

Co-authored-by: PabstMirror <pabstmirror@gmail.com>
@kymckay
Copy link
Member Author

kymckay commented Aug 24, 2021

@Salluci There's a graphic somewhere for the medical rewrite dependency tree, the idea was to split the responsibilities in a logical way.

If I remember right medical engine is the root node and "medical" is the lowest leaf node (not sure if it worked out this way, but the idea was for that component to house public API stuff)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants