-
Notifications
You must be signed in to change notification settings - Fork 70
Add extract_overlay.py and dependency files submodule #3031
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
base: master
Are you sure you want to change the base?
Add extract_overlay.py and dependency files submodule #3031
Conversation
| import argparse | ||
| import multiprocessing | ||
| from pathlib import Path | ||
| from sotn_utils import init_logger, extract |
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.
What's the rationale behind tools/extract_overlay.py being in this repo vs. the other one?
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.
Really just pathing. Seems unnecessary to have it be in a subfolder when it's really just a simple entry point that isn't going to be used anywhere else. Realistically makes very little difference either way.
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.
Having the script in the sotn-decomp repo also establishes a place to address sotn-decomp specific code in an external repo, I just haven't split it up yet.
|
@Xeeynamo I think we should consider removing make-config.py in favour of this I have used it for CAT and ARE now and I think it improves upon a lot of the new config adding experience. |
|
Beyond what was there for cat and are, it now also automatically imports and merges e_init.c (EntityUpdates and EInits) and header.c (it has generated |
|
I’m a bit surprised to see a project-specific utility recreated and relicensed in a separate repository, then proposed as an external dependency rather than contributing improvements upstream. Any changes to make_config.py or closely related tools should also be coordinated with the existing contributors (CC @hohle). From a practical perspective, I’m not sure what the advantage is of pulling I want to avoid the inevitable situations where updating our project's configuration or symbol definitions require opening PRs in a separate repository, waiting for review, and then syncing the changes back. Such friction will only slow us down and make maintenance more complicated. |
This is the primary reason it is in a separate repository, so this doesn't happen to me. I'm updating it incredibly frequently, on the order of daily at times. I do not want everything that is currently in the repo to remain in the repo, though, I want some things to be in sotn-decomp and I am in the process of splitting it up, but that takes time.
I tried having it just be cloned and used independently, but I first created this tool months ago and the only one who used it aside from myself was @JoshSchreuder .
I've brought the tool up twice on discord and have gotten little in the way of overall response there. @JoshSchreuder has been fantastic with feedback and @hohle gave me some good points to adjust (of which I believe I've adjusted all of his concerns). I don't appreciate being told to coordinate when I've done my absolute best to do just that long before submitting the PR. |
I wasn't aware you coordinated with the other contributors. A mention of in the pull request description would have helped me. I am simply sharing my feedback based on my data points, which I have none as your pull request description is blank. |
No description provided.