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

Magicylsm: NPCs can now teach spells #34709

Merged
merged 25 commits into from Oct 20, 2019
Merged

Magicylsm: NPCs can now teach spells #34709

merged 25 commits into from Oct 20, 2019

Conversation

ghost
Copy link

@ghost ghost commented Oct 13, 2019

Summary

SUMMARY: Features "Magicylsm: NPCs can now teach spells"

Purpose of change

Magicylsm: NPCs can now teach spells

Describe the solution

Added a json field for NPC classes, that list spells and levels, like player professions do.
THe NPC then has them added to their "spellbook" when they are spawned.
The NPC spells ar ethen added to the list of things that can be taught in dialogue.
This starts the usual training activity, but with learning a spell at the end of it.

Also fixed a bug in the process that I dont think has been reported, where you can start training with one NPC, then resume it with a different one.

NPCs cant cast spells yet, thats coming, this just lets them know and teach spells.

Describe alternatives you've considered

N/A

Testing

Tested saving/loading with NPC spells, added a debug menu option to print to console what spells each NPC knows.
Tested interrupting and resuming training activity.
Tested time taken for spells to be learned via training
tested spells that are known by the player and spells that arenot.
tested declining to be locked out of wizard type class when training finishes.

Additional context

N/A

@ghost ghost requested a review from KorGgenT October 13, 2019 12:48
@ghost ghost added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mods: Magiclysm Anything to do with the Magiclysm mod NPC / Factions NPCs, AI, Speech, Factions, Ownership Mechanics: Character / Player Character / Player mechanics labels Oct 13, 2019
Copy link
Member

@KorGgenT KorGgenT left a comment

Choose a reason for hiding this comment

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

A couple things: A lot more of these temporary variables can be const, because quite a few of the functions you're using in spell are const.

In addition, if all you need is the name or other values from the spell_type you can simply do spell_id->name.translated() or similar, instead of getting the spell object.

Also, there are a couple of autos hanging around

data/mods/Magiclysm/npc/classes.json Outdated Show resolved Hide resolved
src/dialogue.h Outdated Show resolved Hide resolved
src/npc.cpp Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/npc_class.cpp Outdated Show resolved Hide resolved
src/npc_class.cpp Outdated Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
src/savegame_json.cpp Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
Copy link
Member

@KorGgenT KorGgenT left a comment

Choose a reason for hiding this comment

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

image
looks like the cost isn't getting calculated correctly.

src/npctalk.cpp Outdated Show resolved Hide resolved
src/activity_handlers.cpp Outdated Show resolved Hide resolved
src/npctalk_funcs.cpp Outdated Show resolved Hide resolved
src/npctalk.cpp Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Oct 16, 2019

image
looks like the cost isn't getting calculated correctly.

The cost is 0 when its a follower teaching you.

src/activity_handlers.cpp Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Oct 16, 2019

Merge conflict caused some weirdness , will fix.

This reverts commit fc5c1fe, reversing
changes made to 51f5a66.
src/handle_action.cpp Outdated Show resolved Hide resolved
@KorGgenT KorGgenT merged commit 820aa8d into CleverRaven:master Oct 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Mechanics: Character / Player Character / Player mechanics Mods: Magiclysm Anything to do with the Magiclysm mod NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants