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

Cord teaches One-handed Sword Level 2 too soon #191

Merged
merged 2 commits into from Jul 27, 2021
Merged

Cord teaches One-handed Sword Level 2 too soon #191

merged 2 commits into from Jul 27, 2021

Conversation

AmProsius
Copy link
Owner

@AmProsius AmProsius commented Jul 17, 2021

Describe the bug
Cord's dialog option to teach One-handed Sword Level 2 appears before the player learned Level 1.

Expected behavior
Cord's dialog option to teach One-handed Sword Level 2 now appears only after the player learned Level 1.

Additional context

  • The German translation for One-handed Sword Level 2 is Einhänder Stufe 2.
  • The player can't actually learn Level 2 before Level 1, but for consistency reasons I suggest to hide the dialog option (see #39). Therefore the issue is marked as opinionated.

FUNC int TPL_1402_GorNaToth_TRAINAGAIN_Condition()
{
if (Npc_GetTalentSkill (hero,NPC_TALENT_1H) == 1)
&& (C_NpcBelongsToPsiCamp(hero))
{
return TRUE;
};
};

if (Npc_GetTalentSkill(hero, NPC_TALENT_1H) == 1)
{
Info_AddChoice (DIA_Scatty_TRAIN,B_BuildLearnString(NAME_Learn1h_2, LPCOST_TALENT_1H_2,150) ,DIA_Scatty_TRAIN_2h);
};

@AmProsius AmProsius added validation: validated opinionated labels Mar 12, 2021
@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Mar 12, 2021

FUNC int SLD_709_Cord_TRAINAGAIN_Condition()
{
if (Npc_KnowsInfo (hero,SLD_709_Cord_TRAINOFFER))
&& (Npc_GetTalentSkill (hero,NPC_TALENT_1H) < 2)
{
return TRUE;
};
};

changed to

FUNC int  SLD_709_Cord_TRAINAGAIN_Condition()
{
    if (Npc_KnowsInfo (hero,SLD_709_Cord_TRAINOFFER))
    && (Npc_GetTalentSkill (hero,NPC_TALENT_1H) == 1)
    {
        return TRUE;
    };

};

@AmProsius AmProsius added the provided fix label Mar 12, 2021
@szapp szapp added compatibility: easy impl: hook script func type: session fix labels Apr 5, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Apr 5, 2021

Note on implementation:

Place a hook around the condition function. Before calling the original function, do

if (Npc_GetTalentSkill(hero, NPC_TALENT_1H) != 1) {
    return FALSE;
};

Of course, NPC_TALENT_1H has to be locally defined, retrieved. See similar fixes.

@szapp szapp added this to Dialog: Info condition in Fix templates Apr 5, 2021
@AmProsius AmProsius self-assigned this Apr 17, 2021
@AmProsius AmProsius added this to To Do in v1.2.0 Apr 17, 2021
@AmProsius AmProsius added this to the v1.2.0 milestone Apr 18, 2021
@AmProsius AmProsius removed their assignment Jul 17, 2021
@AmProsius AmProsius requested a review from szapp Jul 17, 2021
@AmProsius AmProsius moved this from To Do to In Progress in v1.2.0 Jul 17, 2021
Copy link
Collaborator

@szapp szapp left a comment

I got some minor remarks. Note: I only skimmed the code and I have not run the tests!

docs/changelog_de.md Outdated Show resolved Hide resolved
@AmProsius AmProsius requested a review from szapp Jul 22, 2021
szapp
szapp approved these changes Jul 22, 2021
Copy link
Collaborator

@szapp szapp left a comment

Code wise it looks good I think. But I haven't tried it. Do all tests pass?

@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Jul 22, 2021

Code wise it looks good I think. But I haven't tried it. Do all tests pass?

Yes, I wrote the test first, tested it, then wrote the fix and tested again. The fist test failed (obviously) and the second test passed.

@AmProsius AmProsius merged commit c7193fb into master Jul 27, 2021
@AmProsius AmProsius deleted the bug191 branch Jul 27, 2021
v1.2.0 automation moved this from In Progress to Done Jul 27, 2021
@szapp szapp moved this from Modify dialog conditions to Add dialog conditions in Fix templates Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility: easy impl: hook script func opinionated provided fix type: session fix validation: validated
Projects
Fix templates
Add dialog conditions
v1.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants