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.

Sign up for GitHub

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

Jump to bottom

harmon patcher continues on error #1076

Merged
merged 25 commits into from
Apr 12, 2021
Merged

harmon patcher continues on error #1076

merged 25 commits into from
Apr 12, 2021

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Apr 9, 2021 •
edited

fixes #1074

  • Harmony patcher continues on error.
  • also it logs which functions have been patched failed patching.
    fixes Logger needs Log.Exception method #1075:
  • I added an extension method to log exceptions. now logging exception is done in a central place and can be made consistent/globally improved.
  • it's not in the log class because I needed CO reference.

https://ci.appveyor.com/api/projects/krzychu124/tmpe/artifacts/TMPE.zip?branch=1074-patcher

depends on PR #1072
blocks PR #1077

@kianzarrin kianzarrin self-assigned this Apr 9, 2021
@kianzarrin
Copy link
Collaborator Author

I removed the patch logs.

kvakvs
kvakvs approved these changes Apr 10, 2021
View reviewed changes
Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine
But please make the error text better (spelling, remove !!!!!!, etc) and report it to user in a readable way.

TLM/TLM/Patcher.cs Outdated Show resolved Hide resolved
@kianzarrin kianzarrin linked an issue Apr 10, 2021 that may be closed by this pull request
patcher to continue on error #1074
Closed
@kianzarrin kianzarrin mentioned this pull request Apr 10, 2021
Closed
kvakvs
kvakvs approved these changes Apr 10, 2021
View reviewed changes
Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

krzychu124
krzychu124 requested changes Apr 10, 2021
View reviewed changes
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review

TLM/TLM/Patcher.cs Show resolved Hide resolved
TLM/TLM/Patcher.cs Outdated Show resolved Hide resolved
@kianzarrin kianzarrin added enhancement Improve existing feature Harmony labels Apr 12, 2021
@kianzarrin kianzarrin removed a link to an issue Apr 12, 2021
patcher to continue on error #1074
Closed
@kianzarrin kianzarrin linked an issue Apr 12, 2021 that may be closed by this pull request
Logger needs Log.Exception method #1075
Closed
@kianzarrin kianzarrin added the Dependent This issue is blocked by another issue. label Apr 12, 2021
@kianzarrin kianzarrin mentioned this pull request Apr 12, 2021
Merged
@kianzarrin kianzarrin added the Blocking Another issue depends on this issue. label Apr 12, 2021
@kianzarrin kianzarrin mentioned this pull request Apr 12, 2021
Merged
krzychu124
krzychu124 approved these changes Apr 12, 2021
View reviewed changes
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok 👍

@chameleon-tbn
Update Patcher.cs
08b2471 
corrected typo :)
chameleon-tbn
chameleon-tbn approved these changes Apr 12, 2021
View reviewed changes
Copy link
Contributor

@chameleon-tbn chameleon-tbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay for me

Base automatically changed from 730-cleanup-loading-CustomPathManager to master April 12, 2021 10:05
@kianzarrin kianzarrin removed the Dependent This issue is blocked by another issue. label Apr 12, 2021
@kianzarrin kianzarrin merged commit f4d55ae into master Apr 12, 2021
@kianzarrin kianzarrin deleted the 1074-patcher branch April 12, 2021 10:26
@originalfoo originalfoo added this to the 11.6.0 milestone Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvakvs kvakvs kvakvs approved these changes

@krzychu124 krzychu124 krzychu124 approved these changes

@chameleon-tbn chameleon-tbn chameleon-tbn approved these changes

@DaEgi01 DaEgi01 Awaiting requested review from DaEgi01

Assignees

@kianzarrin kianzarrin

Labels
Blocking Another issue depends on this issue. enhancement Improve existing feature Harmony
Projects
None yet
Milestone
11.6.0
Development

Successfully merging this pull request may close these issues.

Logger needs Log.Exception method patcher to continue on error
5 participants
@kianzarrin @kvakvs @krzychu124 @chameleon-tbn @originalfoo