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

Conditions in Battles #815

Merged
merged 10 commits into from Mar 14, 2016
@@ -644,7 +644,7 @@ bool Game_BattleAlgorithm::Skill::Execute() {
for (int i = 0; i < (int) skill.state_effects.size(); i++) {
if (!skill.state_effects[i])
continue;
if (!healing || rand() % 100 >= skill.hit)
if (!healing && rand() % 100 >= skill.hit)
continue;
this->success = true;
View
@@ -327,6 +327,40 @@ static bool non_permanent(int state_id) {
return Data::states[state_id - 1].type == RPG::State::Persistence_ends;
}
int Game_Battler::ApplyConditions()
{
int damageTaken = 0;
std::vector<int16_t> inflictedStates = this->GetInflictedStates();
if (inflictedStates.size() != 0) {
for (int i = 0; i<inflictedStates.size(); ++i) {
RPG::State state = Data::states[inflictedStates[i]];

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

The loop can be easier written as:

for (int16_t inflicted : inflictedStates) {
    RPG::State state = Data::states[inflicted - 1];

and the if (inflictedStates.size() != 0) { check is not needed

@Ghabry

Ghabry Mar 12, 2016

Member

The loop can be easier written as:

for (int16_t inflicted : inflictedStates) {
    RPG::State state = Data::states[inflicted - 1];

and the if (inflictedStates.size() != 0) { check is not needed

int hp = state.hp_change_val;
int sp = state.sp_change_val;

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

This calculation ignores the "Percentage of max HP/SP". In other sections of code where this is required we do something like this:

int hp = state.hp_change_val + (int)(std::ceil(GetMaxHp() * state.hp_change_max / 100.0));```
@Ghabry

Ghabry Mar 12, 2016

Member

This calculation ignores the "Percentage of max HP/SP". In other sections of code where this is required we do something like this:

int hp = state.hp_change_val + (int)(std::ceil(GetMaxHp() * state.hp_change_max / 100.0));```
int source_hp = this->GetHp();
int source_sp = this->GetSp();
int src_hp = 0;
int src_sp = 0;
if (state.hp_change_type == state.ChangeType_lose) {
src_hp = -std::min(source_hp + 1, hp);
}
else {
src_hp = std::min(source_hp, hp);
}
if (state.sp_change_type == state.ChangeType_lose) {
source_sp = -std::min(source_sp, sp);
}
else {
source_sp = std::min(source_sp, sp);
}

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

RPG2k3 added RPG::State::ChangeType_gain, which is what you handle in your ELSE case. But RPG::State::ChangeType_none (No change at all) is not handled.

@Ghabry

Ghabry Mar 12, 2016

Member

RPG2k3 added RPG::State::ChangeType_gain, which is what you handle in your ELSE case. But RPG::State::ChangeType_none (No change at all) is not handled.

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

in ChangeType_none case i only need to set src_s/hp = 0?

@Tondorian

Tondorian Mar 12, 2016

Member

in ChangeType_none case i only need to set src_s/hp = 0?

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

Yeah, 0 change

@Ghabry

Ghabry Mar 12, 2016

Member

Yeah, 0 change

this->ChangeHp(src_hp);
this->ChangeSp(src_sp);
damageTaken += src_hp;
}
}
return damageTaken;

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

The return value must be negated (return a negative value for damage currently)

@Ghabry

Ghabry Mar 12, 2016

Member

The return value must be negated (return a negative value for damage currently)

}
void Game_Battler::RemoveBattleStates() {
std::vector<int16_t>& states = GetStates();
View
@@ -58,6 +58,13 @@ class Game_Battler {
*/
std::vector<int16_t> GetInflictedStates() const;
/**
* Apply effects of Conditions to Battler
*
* @return Damage taken to Battler from conditions
*/
int ApplyConditions();
/**
* Gets battler states.
* This returns the raw state list with not inflected states set to 0 and
View
@@ -313,6 +313,10 @@ bool Scene_Battle_Rpg2k::ProcessBattleAction(Game_BattleAlgorithm::AlgorithmBase
switch (battle_action_state) {
case BattleActionState_Start:
if (battle_action_wait--) {
return false;
}
battle_action_wait = 0;
battle_message_window->Clear();
if (!action->IsTargetValid()) {
@@ -358,31 +362,38 @@ bool Scene_Battle_Rpg2k::ProcessBattleAction(Game_BattleAlgorithm::AlgorithmBase
Game_System::SePlay(*action->GetStartSe());
}
battle_action_state = BattleActionState_ConditionHeal;
battle_action_state = BattleActionState_Result;
break;
case BattleActionState_ConditionHeal:
if (battle_action_wait--) {
return false;
}
battle_action_wait = 0;
case BattleActionState_ConditionHeal:
if (action->IsFirstAttack()) {
std::vector<int16_t> states = action->GetSource()->NextBattleTurn();
if (!states.empty()) {
std::vector<int16_t> states_to_heal = action->GetSource()->NextBattleTurn();
std::vector<int16_t> states_remaining = action->GetSource()->GetInflictedStates();
action->GetSource()->ApplyConditions();
if (!states_to_heal.empty() || !states_remaining.empty()) {
battle_message_window->Clear();
for (std::vector<int16_t>::iterator it = states.begin(); it != states.end(); ++it) {
battle_message_window->Push(action->GetSource()->GetName() + Data::states[(*it) - 1].message_recovery);
for (std::vector<int16_t>::iterator it = states_to_heal.begin(); it != states_to_heal.end(); ++it) {
if (!Data::states[(*it) - 1].message_recovery.empty()) {
battle_message_window->Push(action->GetSource()->GetName() + Data::states[(*it) - 1].message_recovery);
}
}
for (std::vector<int16_t>::iterator it = states_remaining.begin(); it != states_remaining.end(); ++it) {
if (!Data::states[(*it) - 1].message_affected.empty()) {
battle_message_window->Push(action->GetSource()->GetName() + Data::states[(*it) - 1].message_affected);
}
}
if (battle_message_window->GetLineCount() != 0) {
battle_action_wait = 30;
}
battle_action_wait = 30;
}
}
if (!action->GetTarget()) {
battle_action_state = BattleActionState_Finished;
}
else {
battle_action_state = BattleActionState_Result;
battle_action_state = BattleActionState_Start;
}
break;
@@ -450,12 +461,12 @@ bool Scene_Battle_Rpg2k::ProcessBattleAction(Game_BattleAlgorithm::AlgorithmBase
}
if (action->TargetNext()) {
battle_action_state = BattleActionState_Start;
battle_action_state = BattleActionState_ConditionHeal;
return false;
}
// Reset variables
battle_action_state = BattleActionState_Start;
battle_action_state = BattleActionState_ConditionHeal;

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

You changed all the battle_action_state transitions. This is super super risky, pls don't do this unless you are 100% certain this is required. Because this must be tested with every available battle action available (Normal, different items, different skills, defend) to proof that it doesn't break anything or added a subtil bug.

One problem I already noticed is, that you introduced some delay before everybodys turn.

@Ghabry

Ghabry Mar 12, 2016

Member

You changed all the battle_action_state transitions. This is super super risky, pls don't do this unless you are 100% certain this is required. Because this must be tested with every available battle action available (Normal, different items, different skills, defend) to proof that it doesn't break anything or added a subtil bug.

One problem I already noticed is, that you introduced some delay before everybodys turn.

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

Yeah i needed to change them to get poisen effect working before the battlers attack.
think need to double check, if waiting is needed

@Tondorian

Tondorian Mar 12, 2016

Member

Yeah i needed to change them to get poisen effect working before the battlers attack.
think need to double check, if waiting is needed

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

I reverted all the state changes and I only took your code from line 370 to 388 and it still works fine for me.

@Ghabry

Ghabry Mar 12, 2016

Member

I reverted all the state changes and I only took your code from line 370 to 388 and it still works fine for me.

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

so it shows conditioneffct, Clean enemyAttacks, Damage clean to you?
Takes me 2h yesterday to get it shown correct

@Tondorian

Tondorian Mar 12, 2016

Member

so it shows conditioneffct, Clean enemyAttacks, Damage clean to you?
Takes me 2h yesterday to get it shown correct

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

In my tests your state changes broke the message box. At the beginning it shows an empty message box with a delay and between battle actions, too.

And after an enemy finished his battle animation the delay got removed.

@Ghabry

Ghabry Mar 12, 2016

Member

In my tests your state changes broke the message box. At the beginning it shows an empty message box with a delay and between battle actions, too.

And after an enemy finished his battle animation the delay got removed.

return true;
}
@@ -792,11 +803,11 @@ bool Scene_Battle_Rpg2k::CheckWin() {
std::stringstream ss;
ss << exp << Data::terms.exp_received;
Game_Message::texts.push_back(ss.str());
ss.str("");
ss << Data::terms.gold_recieved_a << " " << money << Data::terms.gold << Data::terms.gold_recieved_b;
Game_Message::texts.push_back(ss.str());
if (money > 0) {
ss.str("");
ss << Data::terms.gold_recieved_a << " " << money << Data::terms.gold << Data::terms.gold_recieved_b;
Game_Message::texts.push_back(ss.str());
}
for (std::vector<int>::iterator it = drops.begin(); it != drops.end(); ++it) {
ss.str("");
ss << Data::items[*it - 1].name << Data::terms.item_recieved;
@@ -532,6 +532,15 @@ bool Scene_Battle_Rpg2k3::ProcessBattleAction(Game_BattleAlgorithm::AlgorithmBas
if (action->IsFirstAttack() && action->GetStartSe()) {
Game_System::SePlay(*action->GetStartSe());

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

The damageTaken codepath depends on action->GetStartSe()

@Ghabry

Ghabry Mar 12, 2016

Member

The damageTaken codepath depends on action->GetStartSe()

int damageTaken = action->GetSource()->ApplyConditions();
if (damageTaken != 0) {
DrawFloatText(
action->GetTarget()->GetBattleX(),
action->GetTarget()->GetBattleY(),
0,
boost::lexical_cast<std::string>(damageTaken),
30);
}
}

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

Everytime anybody does his turn all allies and enemies get state heal/damage. So should be something like this:

std::vector<Game_Battler*> battlers;
Main_Data::game_party->GetBattlers(battlers);
Main_Data::game_enemyparty->GetBattlers(battlers);

for (auto& b : battlers) {
  // Your damageTaken code
}
@Ghabry

Ghabry Mar 12, 2016

Member

Everytime anybody does his turn all allies and enemies get state heal/damage. So should be something like this:

std::vector<Game_Battler*> battlers;
Main_Data::game_party->GetBattlers(battlers);
Main_Data::game_enemyparty->GetBattlers(battlers);

for (auto& b : battlers) {
  // Your damageTaken code
}

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

Oh there is also an bug in my line should be action->GetSource()->GetBattleX()
but your solution is even better

@Tondorian

Tondorian Mar 12, 2016

Member

Oh there is also an bug in my line should be action->GetSource()->GetBattleX()
but your solution is even better

battle_action_state = BattleActionState_Result;
@@ -834,11 +843,11 @@ bool Scene_Battle_Rpg2k3::CheckWin() {
std::stringstream ss;
ss << exp << Data::terms.exp_received << "\f";
Game_Message::texts.push_back(ss.str());
ss.str("");
ss << Data::terms.gold_recieved_a << " " << money << Data::terms.gold << Data::terms.gold_recieved_b << "\f";
Game_Message::texts.push_back(ss.str());
if (money > 0) {
ss.str("");
ss << Data::terms.gold_recieved_a << " " << money << Data::terms.gold << Data::terms.gold_recieved_b << "\f";
Game_Message::texts.push_back(ss.str());
}
for(std::vector<int>::iterator it = drops.begin(); it != drops.end(); ++it) {
ss.str("");
ss << Data::items[*it - 1].name << Data::terms.item_recieved << "\f";
ProTip! Use n and p to navigate between commits in a pull request.