Skip to content

[game] Implement a shared physical-combat core - #294

Open
vrifftech wants to merge 3 commits into
modawan:masterfrom
vrifftech:master
Open

[game] Implement a shared physical-combat core#294
vrifftech wants to merge 3 commits into
modawan:masterfrom
vrifftech:master

Conversation

@vrifftech

Copy link
Copy Markdown

Summary

This PR establishes one common physical-combat implementation for both supported games. It covers attack results, authoritative attack and Defense inputs, typed damage construction, mitigation, delayed melee impacts, visibility-aware Defense, equipped mitigation, feedback, and damage-event context.

Common runtime path

runtime ability / feat / effect provenance inputs
→ attack result and critical state
→ authoritative Defense
→ source-qualified typed damage packet
→ immunity / resistance / reduction
→ per-impact melee result delivery
→ feedback and object damage-event context

Architectural changes

  • Uses one game-neutral effect provenance representation and source-key encoder.
  • Uses one source/subtype reducer for effective abilities, attack modifiers, and damage modifiers.
  • Uses the common active-effect list for equipped resistance and reduction.
  • Uses one exact queued-user-action target validator.
  • Uses one visibility and counter lifecycle.
  • Uses one melee animation-selection module and one per-impact delivery schedule.

@modawan modawan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good to me overall, thank you! Lots of improvements here.

throw RoutineNotImplementedException("VersusAlignmentEffect");
// The VM validates only the first inherited argument. Supported effect
// families retain both slots; physical-combat consumers compare the
// second with GetSimpleAlignmentGoodEvil.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

GetSimpleAlignmentGoodEvil

There is no such function. Looks like nGoodEvil is handled in effect->appliesVersus(target).

bool damageTypeMatches(int modifierFlags, int damageFlags);

enum class MitigationFeedbackType : uint16_t {
DamageImmunity = 0x3e,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is there any significance to these values? 0x3e, 0x3f, etc.

}
if (slot < static_cast<int>(_context.damageAmounts.size())) {
_context.damageAmounts[slot] = static_cast<int16_t>(amount);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm not sure how this code works. So damage type is a bitmask of electrical, fire, energy, etc.
(flags & (flags - 1)) == 0 seems to check that only 1 bit is set, and then the loop finds the bit number (damage type) and moves all damage to the corresponding bucket (damageAmounts).

What happens when damage type is "universal" (0xFFFF, all bits are 1)?

Comment thread include/reone/game/game.h

std::shared_ptr<Object> getObjectById(uint32_t id) const;

uint32_t lastTarget() const { return _lastTarget; }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What is the purpose of _lastTarget? It seems to be set for any combat attack, object selection, party member add, leader change, but only ever used to inhibit FloatingText::addHeal:

void FloatingText::addHeal(const Object &object, int amount) {
    if (!_game.party().isMember(object) &&
        _game.lastTarget() != object.id()) {
        return;
    }

    add(object, std::to_string(amount), Style::Heal);
}

return;
}
// Native behavior literally restores the Ultravision bit when another
// True Seeing record survives removal. Do not normalize this quirk.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is there anything in the game that depends on this quirk?

@@ -29,7 +32,23 @@ bool DamageImmunityIncreaseEffect::onApply(Object &) {
}

bool DamageImmunityDecreaseEffect::onApply(Object &object) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What Effect::onApply is generally supposed to do? Here it checks that the effect is applied.
However, BonusFeatEffect::onApply actually applies the effect:

bool BonusFeatEffect::onApply(Object &object) {
    auto *creature = dyn_cast<Creature>(&object);
    if (!creature || _feat == FeatType::Invalid) {
        return false;
    }
    creature->addBonusFeat(_feat);
    return true;
}

What is the difference between onApply and applyTo?

Comment thread src/libs/game/game.cpp
}

bool playerTarget = _party.isMember(target) ||
(_party.player() && _party.player()->id() == target.id());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks like the second check is excessive - the player character is also a party member, so isMember should return true.

Comment thread src/libs/game/game.cpp
}

int row = static_cast<int>(_options.game.clientDifficulty);
auto table = getRequiredTwoDA(_services.resource.twoDas, "difficultyopt");

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it is better to read the 2DA file into a structure during initialization. Otherwise we do lookup and validation for every damage event. Also, I guess there are more modifiers that we'll need to use.

Comment thread src/libs/game/object.cpp
damage(amount, damager);
}

int Object::getLastDamageAmount(int damageFlags) const {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

So it is similar to the code in DamageEffect constructor. Splitting damage into individial buckets for each type does not play with this routine it seems. If we keep flags as a bitmask, we can fold this into something like (_lastDamageAmounts.flags & damageFlags) ? _lastDamageAmounts.value : 0

}
}

void Area::refreshEffectInvisibility(Creature &creature) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do we need to run OnNotice script here?

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.

2 participants