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

Group training seminars #51634

Merged
merged 9 commits into from Oct 25, 2021
Merged

Group training seminars #51634

merged 9 commits into from Oct 25, 2021

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Sep 15, 2021

Summary

None

Purpose of change

Fixes #51532

Describe the solution

A new 'C' menu option was added to initiate a training seminar. The player selects a skill to teach, then selects which of their followers they want to participate in the seminar.

  • A skill can only be selected if the player's practical level is >0.
  • A follower can only be selected if their theoretical skill is less than the player's selected skill's level

c_menu
skill_menu
npc_menu
result

Also, dialogue options were added to the NPC talk screen for asking the NPC to host a training seminar, as well as for one-on-one training from the player.

talk_menu4
result

The start_training() function was refactored into start_training_gen(), which accepts one teacher Character and an arbitrary vector of student Characters. This allows one character to train any number of other characters. A new activity ACT_TRAIN_TEACHER was added as well to force the teacher to spend the required time teaching the session.

TODO:

  • Create generic routine to handle training an array of Characters
  • Add conversation logic for initiating a group training seminar (npctalk.cpp)
  • Add conversation dialog tree for new training options (various json)
  • Do not allow student ACT_TRAIN activities to complete without the teacher's ACT_TRAIN_TEACHER activity
  • Adjust training time to reflect reality (roughly 1 hour or more)
  • Add ability to teach proficiencies, martial arts, and spells This would make the menus more cluttered. For now, just add the infrastructure to process these kinds of training.
  • Thorough testing

Describe alternatives you've considered

Testing

Player teaching seminar:

  1. Spawn and recruit a group of NPCs
  2. Start a training seminar through the C menu
  3. Depending on the player's and the NPCs' skill levels, the seminar should take upwards of 1 hour

NPC teaching seminar:

  1. Talk to an NPC friend
  2. Ask the NPC to host a training seminar
  3. Select a number of participating students (including/excluding you)

Additional context

@actual-nh actual-nh added [C++] Changes (can be) made in C++. Previously named `Code` Mechanics: Effects / Skills / Stats Effects / Skills / Stats NPC / Factions NPCs, AI, Speech, Factions, Ownership labels Sep 15, 2021
@actual-nh
Copy link
Contributor

Ping: @eltank?

@PatrikLundell
Copy link
Contributor

If you're going to have one NPC train another you'd have to make sure the one you're not talking to when setting the situation up "knows" about it and so won't continue with a current activity (can either be to break the activity off or not be available for the task while active) and won't be available for a new one (and won't follow the PC around while engaged in a training session).

@eltank
Copy link
Contributor

eltank commented Sep 15, 2021

I personally think that this "near-instantaneous training" is not realistic and should either be removed from the game or replaced with a "training seminar" that takes at least 1h and can have multiple participants.
The current "training" is also practically useless at higher levels (say, above 4) due to how little skill xp it gives vs how much is needed to level up.

My solution to teaching NPCs skills is #51628 (letting them work on practice recipes).

@dseguin
Copy link
Member Author

dseguin commented Sep 15, 2021

I agree that it's not completely realistic. Do you think it would be reasonable to have both crafting practice as well as group training? As in, you could instruct NPCs to craft items or participate in a training seminar?

@eltank
Copy link
Contributor

eltank commented Sep 16, 2021

I agree that it's not completely realistic. Do you think it would be reasonable to have both crafting practice as well as group training? As in, you could instruct NPCs to craft items or participate in a training seminar?

Both of those options sound realistic to me. But if practice via crafting addresses the problem more efficiently it might not be worth the effort of coding seminars. OTOH if it turns out to be easy to code, then why not.

@dseguin
Copy link
Member Author

dseguin commented Sep 17, 2021

Good point. I'll refocus this PR on group training like you described.

@dseguin dseguin force-pushed the teach_npc branch 3 times, most recently from 1d791ab to b8e9ae6 Compare September 20, 2021 04:26
@dseguin dseguin changed the title [WIP] Allow any 2 characters to train each other [WIP] Group training seminars Sep 20, 2021
@dseguin dseguin force-pushed the teach_npc branch 2 times, most recently from 75732c8 to 83b80b3 Compare September 27, 2021 02:23
@dseguin
Copy link
Member Author

dseguin commented Sep 27, 2021

Ping: @eltank
If possible, could you tell me what you think of the updated solution? (Screenshots in PR description)

@PatrikLundell
Copy link
Contributor

I believe training sessions ought to take time rather than being instantaneous with spamming blocked by a "memory" of having trained someone. If seminars (or, for that matter one on one training) takes time there's no need for a timeout timer.
If instantaneous training remains (due to implementation difficulties), the timeout would probably have to remain until that's been sorted out.

The snipped of changed code seems to imply there's a check for if the player has been asked to train ("player_character"), but it really should be a check for the teacher having been asked to train, regardless of whether the teacher is the (current) player character or not.

@dseguin
Copy link
Member Author

dseguin commented Sep 27, 2021

I'm sorry, I don't really understand. Are you saying that training is currently instantaneous? That shouldn't be the case. I'm using the same mechanism as receiving training from an NPC, so it's handled through an activity. It takes roughly 50-100 turns depending on the skill requirements. (see talk_function::start_training_gen())

Yeah, currently the asked_to_train effect is checked on the player because only the player can teach a seminar through the C menu. I found that it's a bit awkward to add another submenu to the C menu to cycle through and select an NPC as the teacher. I think that option would work better in the specific NPCs dialogue tree (same place as the current training option).

@PatrikLundell
Copy link
Contributor

Training currently is very quick, although it might not be instantaneous. 50-100 seconds is extremely fast, whereas I'd expect a "real" training session to be somewhere around 45-60 minutes, based on how long individual lessons tend to be currently.

I thought there was a reason for the specification of the player character, but I don't think it hurts to check. Also, if it's reasonably easy to implement, having the function just take a character would make it possible to reuse it later on when/if the UI issues have been sorted out.

@dseguin
Copy link
Member Author

dseguin commented Sep 27, 2021

Yes I agree, eltank mentioned earlier that he wanted training activities to take >1h. I've added it to the TODO checklist above.

Also, if it's reasonably easy to implement, having the function just take a character would make it possible to reuse it later on

That's how I implemented it:

void talk_function::start_training_gen( Character &teacher, std::vector<Character *> &students,
teach_domain &d )

@dseguin dseguin force-pushed the teach_npc branch 2 times, most recently from 900d4f9 to 82a490a Compare October 2, 2021 02:38
@dseguin dseguin changed the title [WIP] Group training seminars Group training seminars Oct 2, 2021
@dseguin dseguin changed the title Group training seminars [CR] Group training seminars Oct 3, 2021
@dseguin dseguin marked this pull request as ready for review October 4, 2021 17:47
@dseguin dseguin force-pushed the teach_npc branch 4 times, most recently from 7a5730d to 339f0e6 Compare October 12, 2021 05:15
src/npctalk.cpp Outdated Show resolved Hide resolved
@dseguin dseguin changed the title [CR] Group training seminars Group training seminars Oct 22, 2021
@kevingranade kevingranade merged commit d65732d into CleverRaven:master Oct 25, 2021
@dseguin dseguin deleted the teach_npc branch November 7, 2021 01:18
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` Mechanics: Effects / Skills / Stats Effects / Skills / Stats NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Teach NPCs skills
6 participants