fix: move tamed pet to proper faction set in creature_tracker layer

[ekaratzas]

Purpose of change

The reason for this PR is me hitting the following error message in the debug build as explained in the respective task when trying to tame a pet: "ERROR: the chicken tried to find faction player which wasn't loaded in game::monmove"

Fixes #4284

Describe the solution

Digging into it, it seems that once the animal gets tamed with food, it's never moved to the 'player' faction set of Creature_tracker::monster_faction_map_ and then we hit the error in monster::plan() in monmove.cpp.

What this change does is move the existing code that 'makes the animal a pet' into a new method, monster::make_pet(), akin to similar functions in that class. It also introduces a helper function in creature_tracker layer that allows us to notify that layer that there has been a change in the faction that this monster belongs to.

In the new method of Creature_tracker::update_faction() layer we remove the monster from existing sets in monster_faction_map_ and we use the existing internal helper function add_to_faction_map() to properly add the pet in the appropriate set.

Describe alternatives you've considered

Looked into CDDA repo but the creature_tracker layer has been majorly overhauled/rewritten. Porting a fix from there didn't seem like the way to go.

Testing

Follow repro steps. Spawn dog/chickens. Try to tame them. Notice we don't hit this error anymore.

Additional context

Checklist

[scarf005]

hi, thanks for the PR. i have two quick questions unrelated to PR:

1. did you join the BN discord server (wasn't able to find by github name)?
2. would it be possible/make sense to unify behavior of make_ally and make_pet into one, e.g like this?

        /** Makes this monster an ally of the given creature. */
        void make_ally( const Creature &c );

[scarf005]

LGTM

[ekaratzas]

hi, thanks for the PR. i have two quick questions unrelated to PR:

1. did you join the BN discord server (wasn't able to find by github name)?

2. would it be possible/make sense to unify behavior of `make_ally` and `make_pet` into one, e.g like this?

        /** Makes this monster an ally of the given creature. */
        void make_ally( const Creature &c );

Hey @scarf005,

I'm not in the discord yet. Mom warned me against talking to strangers. But I can join if you think it'd help facilitate technical conversations.

Good question about make_ally()! I saw that the player class does not inherit from monster but it is true that both of them have Creature in their inheritance tree so we could potentially rewrite it as void monster::make_ally( const creature& ). But we'd still probably have to have 2 branches in the new combined make_ally(), one if the parameter is the player, another if it's another monster.

One thing that really weirded me out is that the code in iuse.cpp that turned the animal into a pet sets monster.friendly to -1 instead of a positive value, friendly being an int. Whereas, make_friendly() sets it to a random positive value and make_ally just inherits the value. I guess it might be supposed to be an identifier? :-S I don't yet quite grasp if there's a fine technical reasoning on why the allied 'pet' needs to have -1 instead of a positive value but I maintained this behavior in this PR as you can see for the time being.

I'd have to look up usage of 'friendly' throughout the codebase to figure out how it's being used and what we should be doing with it. For example in the creature_tracker layer we just check if monster.friendly != 0 so it doesn't really matter there what the actual friendly value is as long as it's not 0. Weird stuff.