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

Character behaviors to allow monsters to target characters #309

Merged
merged 17 commits into from Nov 10, 2020

Conversation

Hypexion
Copy link
Contributor

@Hypexion Hypexion commented Nov 8, 2020

This enables the MonsterTarget and MonsterTargetIfPlayerIs behaviors, and also adds the CanBeNPCBeethro behavior.

Rather than doing any complicated, setting behaviors that make a character into a monster target simply adds them to the list of monster enemies in CDbRoom. Similarly, deactivating them will remove them from the list. Changes have been made in the appropriate places so that character monster enemies are removed from the list when killed, or when the room object is cleared. A small change in CMonster::Move() has also been made to allow characters that can be attacked by monsters to be killed by monsters. This replaces a previous identity check.

Neither monster target behavior is set by default for any character type, as it was not previously possible for most characters to be monster targets.

In order to preserve old functionallity after changes to CCharacter::IsMonsterTarget(), the behavior CanBeNPCBeethro has been added. This is now used when finding the "NPC Beethro" during monster target searchs. It only has an effect on Beethro and Gunthro NPCs, which have this behavior by default, and is not really intended to used by architects, although it is still exposed via the UI. Suggestions for naming and documentation of this behavior are welcome.

Copy link
Member

@mrimer mrimer left a comment

Choose a reason for hiding this comment

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

This looks good!

I made some naming suggestions, plus typo fixes.

Beyond these, I'm not sure I'm understanding whether there are any backwards compatibility issues. Can you confirm that all pre-existing holds should continue to function exactly the same as before with the addition of all the 5.2 behaviors?

DRODLib/Character.cpp Outdated Show resolved Hide resolved
DRODLib/Character.cpp Show resolved Hide resolved
Data/Help/1/script.html Outdated Show resolved Hide resolved
Data/Help/1/script.html Outdated Show resolved Hide resolved
@@ -162,7 +162,8 @@ namespace ScriptFlag
PuffImmune = 21,
ActivatePlates = 22,
PushMonsters = 23,
FatalPushImmune = 24
FatalPushImmune = 24,
Copy link
Member

Choose a reason for hiding this comment

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

I realize I didn't understand the semantic of "MonsterTargetIfPlayerIs" before.

The behavior text "Monster Target If Player Is Monster Target" and documentation helps explain this. Suggest there is a comment added to code, maybe at enumeration #7, explaining what this means.

Alternatively, I have a couple possible naming suggestions, based on how the logic operates.

Option 1. MonsterTargetIfPlayerIsTarget -- reading the original name, at first I was thinking, "If the player is ... what?" I didn't realize "a target" is what was meant. I thought there might be a parameter for what the player is. Being explicit in expanding the phrasing helps resolve the ambiguity in reading the name. To me, it's okay to make these names long if it helps with clarity.

Option 2. MonsterTargetWhenPlayerIsTarget -- I think changing "if" to "when" helps clarify that there is a synchronization aspect to how this behavior works, i.e., the NPC and player being targets are both true or false at the same time.

Option 3. MonsterTargetOncePlayerIsTarget -- if it's impossible for a player to stop being a monster target once in the room they become a target, this name might be clearer. It helps the architect understand that this behavior gets triggered as soon as the player is flagged as a target, and the behavior will stick for the remainder of the room (unless the behvior setting is changed, of course).

Leaning toward option #3 if I'm understanding correctly how the logic works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think option 2 is best, since it's possible for the player to regain stealth using the Set Player Stealth command. Since it's possible to "go backwards", using when best communicates the syncronization, as you said.

@Hypexion Hypexion changed the base branch from tss-5.2-dev to rpg-1.3-dev November 10, 2020 14:35
@Hypexion Hypexion changed the base branch from rpg-1.3-dev to tss-5.2-dev November 10, 2020 14:36
@Hypexion
Copy link
Contributor Author

Beyond these, I'm not sure I'm understanding whether there are any backwards compatibility issues. Can you confirm that all pre-existing holds should continue to function exactly the same as before with the addition of all the 5.2 behaviors?

I'm fairly certain that none of these behaviors should cause compatibility problems, but it's possible I've forgotten something. For the monster targeting behaviors specifically, they need to be enabled to make the character a target, and no appeaance has them behaviors by default.

For other behaviors, whenever I've replaced a check on the characters apparance (or other qualities), I've added code to CCharacter::SetCurrentGame() to give characters with eligable appearances the behaviors required to continue old behavior. So tarstuff baby characters get the Hot Tile Immune behavior, Construct characters get Drop Trapdoors and Push Monsters, and human appearances get all the stuff that was previously restricted to human characters.

@mrimer mrimer merged commit 7f0282a into CaravelGames:tss-5.2-dev Nov 10, 2020
@Hypexion Hypexion deleted the target-behaviors branch November 10, 2020 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants