[Anchor] Add PVP damage multiplier #6011
Conversation
flee135
force-pushed
the
anchor-pvp-mult
branch
from
f344afc to
3f0a3a5
Compare
flee135
force-pushed
the
anchor-pvp-mult
branch
from
3f0a3a5 to
30046a1
Compare
|
|
||
| roomState.ownerClientId = payload["state"]["ownerClientId"].get<uint32_t>(); | ||
| roomState.pvpMode = payload["state"]["pvpMode"].get<u8>(); | ||
| roomState.pvpDamageMult = payload["state"]["pvpDamageMult"].get<u8>(); |
There was a problem hiding this comment.
If this client joins a room that is hosted by an older client (without this setting) this will result in a crash. Could you do a conditional check here? I think there is probably some examples somewhere, if not here then in the older anchor code
There was a problem hiding this comment.
Thanks, good catch. Changed it to not explicitly using a conditional, but instead calling .value() to use value of pvpDamageMult if it exists in the payload, or a default of 1 if it doesn't, which should correspond to the old 2x behavior.
There was a problem hiding this comment.
Good find, I'll probably migrate to using this in a few other places as well.
garrettjoecox
left a comment
garrettjoecox
left a comment
There was a problem hiding this comment.
Pending the backwards compatibility check LGTM
Call .value() to use pvpDamageMult if present in UpdateRoomState packet, or default to 1. Prevents crashes when connecting with old clients without the new field.
|
I'd rather not have such long texts inside the dropdown like that. Not sure how it could be done, since a tooltip would get pretty big, too, but such long lines would cause issues for smaller screens anyway. Tooltip, at least, would be better than it is now, though. |
They are shorter than the screenshot after the options were reduced, what do you think? |
|
Ah, yeah, that's much smaller than the screenshot, which was what I was working off of. |
Whoops, sorry I missed this, I'm currently traveling for the holidays and don't have a machine setup to build ship right now. If it's still a problem, I can try to set that up and try moving it to a tooltip and have the dropdown only include the multipliers. Or I can revisit this in mid-January when I'm back from travel. |
| u8 damage = payload["damage"].get<u8>(); | ||
|
|
||
| self->actor.colChkInfo.damage = damage * 8; // Arbitrary number currently, need to fine tune | ||
| self->actor.colChkInfo.damage = damage * GetPvpDamageMultiplier(roomState) * 4; |
There was a problem hiding this comment.
this is a u8, with OHKO you're going to overflow it
passing damage as parameter to GetPvpDamageMultiplier you could have OHKO ignore damage value & return 63 (need to avoid overflow from * 4). But I'm also thinking might as well push the * 4 down too, renaming GetPvpDamageMultiplier to TransformPvpDamage
4 participants
Adds dropdown in anchor menu for admin to adjust PvP damage multipliers. Defaults to 2x, which is the current hardcoded behavior.
Could use feedback on the text descriptions for each multiplier. Not sure if it's too lengthy, unnecessary to include, or if it belongs in a tooltip.
Build Artifacts