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

Fix for whip/pike attack invisible but revealed enemies #540

Closed

Conversation

brturn
Copy link
Contributor

@brturn brturn commented Mar 10, 2023

As theorized by @slow-burn on brogue discord, the pike, whip, and flail do not work against invisible but revealed enemies (eg. telepathy).

  • Pike & spear work
  • Flail works
  • Axe works
  • Rapier works
  • Whip works against invisible enemies
  • Whip cannot attack submerged enemies

@brturn brturn marked this pull request as draft March 11, 2023 00:03
@brturn brturn marked this pull request as ready for review April 29, 2023 21:49
Copy link
Owner

@tmewett tmewett left a comment

Choose a reason for hiding this comment

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

Thanks!

Thought this was making hitlist inclusion and attack triggering use the same code, but I see that's only for the flail where that logic actually should be the same.

Just a question.

src/brogue/Monsters.c Outdated Show resolved Hide resolved
@@ -3460,15 +3460,13 @@ void getImpactLoc(pos *returnLoc, const pos originLoc, const pos targetLoc,
pos coords[DCOLS + 1];
short i, n;
creature *monst;
creature *orig = monsterAtLoc(originLoc.x, originLoc.y);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps *attacker or *caster instead of *orig ?

@@ -1,2 +1,2 @@
-
The rapier lunge attack works against invisible enemies when you are telepathic.
The whip, spear, and rapier now work against invisible enemies when you are telepathic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also mention that whips no longer work against submerged enemies?

Copy link
Owner

@tmewett tmewett left a comment

Choose a reason for hiding this comment

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

We should name and document canAttack properly before merging


n = getLineCoordinates(coords, originLoc, targetLoc, theBolt);
n = min(n, maxDistance);
for (i=0; i<n; i++) {
monst = monsterAtLoc(coords[i].x, coords[i].y);
if (monst
&& !monsterIsHidden(monst, monsterAtLoc(originLoc.x, originLoc.y))
if (canAttack(orig, monst)
Copy link
Owner

Choose a reason for hiding this comment

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

This is wrong from the POV of getImpactLoc. Bolts do not pass through creatures if the caster is unable to attack them.

Comment on lines +233 to +235
// Centralize the logic of whether an attacker can/will attack the defender
boolean canAttack( creature *attacker, creature *defender ) {
Copy link
Owner

Choose a reason for hiding this comment

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

We should decide what this means precisely. For example, this check includes that the defender's location is known, but it's obviously possible to attack completely invisible enemies.

It probably means something more like "does the attack know it will attack defender if it moves into its cell?"

The player case was equivalent to `canSee || revealed` because the inner
`|| revealed` in `canSee` is made redundant by the outer one.
@brturn
Copy link
Contributor Author

brturn commented May 19, 2024

Closing in favor of #686, which is much cleaner and can be applied universally.

@brturn brturn closed this May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants