-
Notifications
You must be signed in to change notification settings - Fork 260
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
perf(port): Optimization tweaks #4634
Conversation
Co-Authored-By: Zhilkin Serg <ZhilkinSerg@users.noreply.github.com> Co-Authored-By: Chaosvolt <chaosvolt@users.noreply.github.com>
Autofix has formatted code style violation in this PR. I edit commits locally (e.g: git, github desktop) and want to keep autofix
I do not want the automated commit
If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT. |
Co-Authored-By: Zhilkin Serg <ZhilkinSerg@users.noreply.github.com> Co-Authored-By: Chaosvolt <chaosvolt@users.noreply.github.com> Co-Authored-By: VissValdyr <167661460+VissValdyr@users.noreply.github.com> Co-Authored-By: Olanti <olanti-p@yandex.ru> Co-Authored-By: Coolthulhu <Coolthulhu@gmail.com>
…lysmbnteam/Cataclysm-BN into optimizations_port_update1
Okay so evidently #4636 contained the file changes for this PR and the other one so these can be closed? According to Kenan at least. I'm going to hope this is the right decision, if anyone spots anything vital in this re-open and fix merge conflicts then, bleh. |
Okay so it turns out merging that other PR was a mistake for reasons entirely unrelated to splitting them weirdly and instead just because the source PRs in #4635 specifically need more time in the oven before we want to port them, so I think this one specifically is 100% good to reopen after the revert, then we need to go on to figure out if anything in #4635 can be separated from the bad keybind code changes and salvaged, then lastly grab anything unique to #4635 as its own PR. |
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.
Compiled and load-tested, seems good at first glance. More importantly, no keybind errors so I can confirm that nothing critical from #4635 snuck into this one.
That said I'd like to leave this for Coolthulhu, Scarf, or Kheir to look at and make sure they give it their approval before any merges.
Closing this because pull request of @KheirFerrum includes fixed version of this work |
Ports this pull request
CleverRaven/Cataclysm-DDA#40961,
This also includes pull requests #4627, #4623 by @chaosvolt for easier mergeability
activity_handlers.cpp
file, line 1257 throws an error during compiling which, based on conversation with @Coolthulhu, will be fixed with help of Coolthulhu or other developers or bothCoauthors have been linked properly via GitHub Desktop built-in function to do this work hopefully