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

Jump to bottom

Guard has duplicated END dialog #244

Merged
merged 9 commits into from Jul 28, 2021
Merged

Guard has duplicated END dialog #244

merged 9 commits into from Jul 28, 2021

Conversation

AmProsius
Copy link
Owner

@AmProsius AmProsius commented Jul 22, 2021 •
edited

Describe the bug
One of the guards guarding the ore barons' house in the Old Camp has two END dialog options.

Expected behavior
One of the guards guarding the ore barons' house in the Old Camp no longer has two END dialog options.

Additional context
The NPC is GRD_245_Gardist. This issue was created by splitting #42 in two separate issues.

@AmProsius AmProsius added the validation: required This issue needs validation from one of the validators. label Jul 22, 2021
@AmProsius AmProsius mentioned this pull request Jul 22, 2021
Closed
@AmProsius AmProsius added compatibility: easy This issue is easy to make compatible. impl: hook script func This issue requires hooking script functions. impl: modify/analyze script func This issue requires analyzing and/or modifying the bytecode of script functions. provided fix This issue has a fix provided in the comments. type: session fix The fix for this issues is persistent across a session. validation: validated This issue is still present even with Systempack/Union. and removed validation: required This issue needs validation from one of the validators. labels Jul 22, 2021
@AmProsius AmProsius added this to In Progress in v1.2.0 via automation Jul 22, 2021
@AmProsius AmProsius added this to the v1.2.0 milestone Jul 22, 2021
@szapp szapp mentioned this pull request Jul 24, 2021
Merged
@szapp
Copy link
Collaborator

szapp commented Jul 24, 2021

See requested changes in #128.

szapp
szapp requested changes Jul 24, 2021
View reviewed changes
Copy link
Collaborator

@szapp szapp left a comment

Choose a reason for hiding this comment

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

Here are some additional changes. Since this PR is based on the branch of another PR, I am not sure what's the best course of action. Possibly, it's best to fix the things in #128 first, merge that PR and then, in turn, merge master into this PR here, before fixing the things here.

src/Ninja/G1CP/Content/Fixes/Session/fix042_GuardExitDialog.d Outdated Show resolved Hide resolved
src/Ninja/G1CP/Content/Fixes/Session/fix128_GuardExitDialog.d Outdated Show resolved Hide resolved
src/Ninja/G1CP/Content/Fixes/Session/fix244_GuardExitDialog.d Outdated Show resolved Hide resolved
src/Ninja/G1CP/Content/Tests/test244.d Outdated Show resolved Hide resolved
@AmProsius AmProsius requested a review from szapp July 27, 2021 08:21
szapp
szapp approved these changes Jul 27, 2021
View reviewed changes
Copy link
Collaborator

@szapp szapp left a comment

Choose a reason for hiding this comment

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

Looks great! I haven't run it or the tests.

One thing that may have gotten complicated from the back and forth: Do the waypoints in tests correspond to the correct NPC of the particular fix (i.e. #42 is the left guard and #244 is the right guard)? Just to make sure.

docs/changelog_de.md Outdated Show resolved Hide resolved
@AmProsius
Copy link
Owner Author

Do the waypoints in tests correspond to the correct NPC of the particular fix (i.e. #42 is the left guard and #244 is the right guard)? Just to make sure.

Yes, I tested all three fixes after the changes.

@AmProsius AmProsius merged commit 97ebf81 into master Jul 28, 2021
v1.2.0 automation moved this from In Progress to Done Jul 28, 2021
@AmProsius AmProsius deleted the bug244 branch July 28, 2021 14:56
@szapp szapp added this to Dialog: Condition in Fix templates Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szapp szapp szapp approved these changes

Assignees
No one assigned
Labels
compatibility: easy This issue is easy to make compatible. impl: hook script func This issue requires hooking script functions. impl: modify/analyze script func This issue requires analyzing and/or modifying the bytecode of script functions. provided fix This issue has a fix provided in the comments. type: session fix The fix for this issues is persistent across a session. validation: validated This issue is still present even with Systempack/Union.
Projects
Fix templates
Modify dialog conditions
v1.2.0
  
Done
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AmProsius @szapp