Refactor npctalk.h to avoid dynamic_cast

[hexagonrecursion]

Summary

None

Purpose of change

Fixes #55626

90f84f4 introduced a dynamic_cast in npctalk.h. This triggered a false positive warning from gcc11. It also is bad c++ style - a vritual method is clearly a better approach here.

Describe the solution

Add a new virtual method so that we no longer need to dynamic_cast

Describe alternatives you've considered

• Tweak the code until gcc11 is no longer confused
• Disable the warning

Testing

• TODO: I did not test this patch.

Additional context

[anothersimulacrum]

FYI #55609

[haveric]

Funnily enough, I did consider going this route instead in the initial change, but wasn't sure the right way to go about it and didn't want to pollute character unnecessarily. Good to know that this is the better approach.

[hexagonrecursion]

Good to know that this is the better approach.

It is just my opinion that this is better. The use of virtual methods is considered idiomatic in C++ and dynamic_cast is considered an anti-pattern.

[haveric]

It is just my opinion that this is better.

I don't write C++ outside of the work I have done for this project, so I am very much relying on the opinions of others who I assume know more here. I knew that it probably should share the code, but didn't know the right way to handle the npc/character functions here. Any/all changes to this are helpful to me in learning how C++ can handle this kind of situation and the potential downsides of what I initially had. Even knowing that I should try to avoid dynamic_cast if I can is still helpful.

[hexagonrecursion]

Strange. I know dynamic cast is to be avoided. This is what I have learned years ago. What I do not remember is: why.

[kevingranade]

The reason is dynamic_cast subverts the intended encapsulation and information hiding that is the primary goal of OO.

A piece of code that has a reference to a creature should never have to care what kind of creature it is, just exercise the interface and have the behaviors adjust as needed depending on the underlying type. dynamic_cast and friends subverts this and makes the code harder to read and reason about, special cases proliferate.

The other fix moved the definition of the method, which is also an improvement, if you don't mind rebasing on top of that change I'd still like to have the change you made here.

[hexagonrecursion]

rebasing on top of that change

Will do.