[3.x] WIP: Initial Action and StatusEffect implementation #958
Conversation
[3.x] Some initial action work (mostly statuses)
[3.x] More action work, begin implementing jobs
[3.x] Move modifier logic to StatusEffect, add parry, remove wrath
| { | ||
| case ClassJob::Warrior: | ||
| { | ||
| Warrior::onAction( *m_pSource->getAsPlayer(), *this ); |
There was a problem hiding this comment.
i'm okay with this as is for now, but i think it would probably better to have an array or otherwise that's just indexed by classjob (which also lets you pick up the base classjob impl easy) instead of this - i get this is wip-ish though so i'm not too worried about it at the moment
| @@ -430,7 +430,7 @@ namespace Excel | |||
| uint8_t HideCastBar : 1; | |||
| uint8_t IsTargetLine : 1; | |||
|
|
|||
| int8_t padding0; | |||
| int8_t unknown : 8; | |||
There was a problem hiding this comment.
why : 8 ? it is already an 8 bit value.
| actor.removeSingleStatusEffectById( WrathII ); | ||
| actor.removeSingleStatusEffectById( WrathIII ); | ||
| actor.removeSingleStatusEffectById( WrathIV ); | ||
| actor.removeSingleStatusEffectById( Infuriated ); |
There was a problem hiding this comment.
it might be better to have something that can remove a list of ids if that is required.
| @@ -40,6 +40,8 @@ struct StatusModifier | |||
| struct StatusEntry | |||
| { | |||
| uint16_t id; | |||
| int32_t duration; | |||
There was a problem hiding this comment.
Can the duration be negative? If not, unsigned makes more sense.
| @@ -583,13 +590,72 @@ void Action::Action::buildActionResults() | |||
| shouldRestoreMP = false; | |||
| } | |||
| } | |||
|
|
|||
| // If we hit an enemy | |||
| if( m_hitActors.size() > 0 && getHitChara()->getObjKind() != m_pSource->getObjKind() ) | |||
There was a problem hiding this comment.
i'd prefer !m_hitActors.empty(), it is more elaborate
|
|
||
| handleJobAction(); | ||
|
|
||
| if( m_lutEntry.statuses.caster.size() > 0 || m_lutEntry.statuses.target.size() > 0 ) |
There was a problem hiding this comment.
same thing here. !empty() is cleaner than size() > 0
| @@ -89,6 +101,19 @@ void ActionResult::applyStatusEffectSelf( uint32_t id, int32_t duration, uint8_t | |||
| m_pStatus->setParam( param ); | |||
| } | |||
|
|
|||
| void ActionResult::applyStatusEffectSelf( uint32_t id, int32_t duration, uint8_t param, std::vector< World::Action::StatusModifier > modifiers, | |||
There was a problem hiding this comment.
as mentioned, i think it would be good to not pass a copy of the vector here.
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a map, why do you iterate it to find an id? You should be able to jund "find()" it.
| { | ||
| if( effectIt.second->getId() == id ) | ||
| return effectIt.second; | ||
| } |
There was a problem hiding this comment.
same thing here, of there is no proper indexing for it, it might be worth to create such a lookup. iterating the entire list all the time might be overkill.
| return m_flag; | ||
| } | ||
|
|
||
| std::vector< World::Action::StatusModifier > Sapphire::StatusEffect::StatusEffect::getStatusModifiers() const |
There was a problem hiding this comment.
this should probably return a const ref
Still a WIP, but can be useful for some other things, so submitting this now.
Development will be done on
want_some_action.