-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix NPC dialog around lying and succeeding mission #27972
Conversation
jenkins rebuild |
1 similar comment
jenkins rebuild |
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.
Minor fixes, but thanks for looking into this.
"effect": "clear_mission", | ||
"mission_opinion": { "trust": 4, "value": 3 }, | ||
"opinion": { "fear": -1, "anger": -1 } | ||
"success": { "topic": "TALK_NONE", "mission_opinion": { "trust": 4, "value": 3 }, "opinion": { "fear": -1, "anger": -1 } } |
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.
There's no trial here, so the previous version should have been fine.
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.
I debugged the code when applying this, and without putting the opinion and mission_opinion into a success block, the JSON parsing does not pick them up at all. This change absolutely affected the outcome.
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.
Okay, good to know. I'll fix that at some point.
}, | ||
{ | ||
"text": "Glad to help. I need no payment. Bye!", | ||
"topic": "TALK_DONE", | ||
"effect": "clear_mission", | ||
"mission_opinion": { "trust": 4, "value": 3 }, | ||
"opinion": { "fear": -1, "anger": -1 } | ||
"success": { "topic": "TALK_DONE", "mission_opinion": { "trust": 4, "value": 3 }, "opinion": { "fear": -1, "anger": -1 } } |
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.
Same here.
Implemented changes mlangsdorf suggested on review. references CleverRaven#27972
gorgon complains about the format of the JSON references CleverRaven#27972
jenkins rebuild |
Apparently I had an extra newline at the end of the JSON file. This removes that. Also I had to create a solution to compile the format.cpp in Visual Studio, but I'm not checking that in at the moment unless others request me to. references CleverRaven#27972
Typo in mission effect caused JSON parsing to fail. references CleverRaven#27972
JSON refactoring for dialog changed some outcomes and chances with the mission dialogs. This brings the chances and results back in line with what they were before the JSON refactor. Fixes CleverRaven#27971
1c72a54
to
a11526b
Compare
Putting the clear_mission effect into the success section of the JSON seems to be required. Also the clear_mission effect was removing a mission reference before the opinion modifiers could be applied. Also, choosing training for a mission reward will automatically clear the mission, and if the JSON forces it to clear with an effect, the player has to pay for the training. references CleverRaven#27972 references CleverRaven#27983
SUMMARY
SUMMARY: Bugfixes "Fixed NPC dialog around lying and succeeding at missions"
Purpose of change
JSON refactoring for dialog changed some outcomes and chances with the
mission dialogs. This brings the chances and results back in line with
what they were before the JSON refactor.
Fixes #27971
Describe the solution
Added opinion changes where they were missed, changed the structure of the JSON to actually pick up opinion changes when dialog is executed. Removed incorrect dialog effect for train reward.
Describe alternatives you've considered
No particular alternative considered.
Additional context
These fixes turned out to be entirely JSON edits, which I think is awesome. Also, the JSON refactoring is making the main talk code a bit more readable.