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

Enemy AI changes: Change weighting of priorities and add special check #2378

Closed
wants to merge 1 commit into from
Closed

Enemy AI changes: Change weighting of priorities and add special check #2378

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 4, 2020

This PR tries to fix #1831.

This one changes the weighting of the priorities: The action with the highest priority always gets a weight of 10 and actions with a priority of (highest priority - 9) gets a weight of 1. The scaling between is linear. This means if we have two actions, one with a
priority of 50 and one with a priority of 45, the first action gets a 10/15 chance and the second action a 5/15 chance to get selected. Moreover a special check has been added if the enemy uses a skill which only heals states without affecting anything else: He actually checks if there is an ally which is affected by at least one state which get healed by the skill. If this is not the case, then the action gets discarded.

@mateofio
Copy link
Contributor

mateofio commented Oct 5, 2020

This should go after #2380

@fdelapena fdelapena added this to the 0.6.3 milestone Oct 5, 2020
@fdelapena fdelapena added Has PR Dependencies This PR depends on another PR Awaiting Rebase Pull requests with conflicting files due to former merge labels Oct 5, 2020
The weighting of the priorities has been changed: The action with the
highest priority always gets a weight of 10 and actions with a
priority of (highest priority - 9) gets a weight of 1. The scaling
between is linear. This means if we have two actions, one with a
priority of 50 and one with a priority of 45, the first action gets
a 10/15 chance and the second action a 5/15 chance to get selected.
Moreover a special check has been added if the enemy uses a skill
which only heals states without affecting anything else: He actually
checks if there is an ally which is affected by at least one state
which get healed by the skill. If this is not the case, then the
action gets discarded.
@ghost
Copy link
Author

ghost commented Oct 5, 2020

This PR is still intact - a rebase is not needed after merging #2380. The IsSkillUsable function remains unchanged because the AI makes an extra check besides IsSkillUsable which is put into an own function (which almost always returns true except for skills which heals states only without affecting anything else).

@@ -244,3 +247,65 @@ const lcf::rpg::EnemyAction* Game_Enemy::ChooseRandomAction() {
return nullptr;
}

bool Game_Enemy::IsSkillUsableForAI(int skill_id) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

RPG_RT does this by calling a function called RPG_Battler::IsSkillEffectiveOn which does a set of checks on each battler. The logic you have here is close but missing some edge cases. Also I need to do more testing to be sure.

@mateofio
Copy link
Contributor

mateofio commented Oct 5, 2020

You've made some good improvements and found some bugs here, but to be honest this is another one like autobattle where it can never be perfect with black box testing. We really need to use RE tools to discover all the edge cases.

EnemyAI is next on my list to do as I also want to make a standalone factory function for it with customizable algos, just like autobattle.

@ghost
Copy link
Author

ghost commented Oct 5, 2020

Okay, for now I wait for further directions from you regarding this PR to avoid merge conflicts. 👍

@fdelapena fdelapena removed the Awaiting Rebase Pull requests with conflicting files due to former merge label Oct 6, 2020
@ghost
Copy link
Author

ghost commented Oct 9, 2020

@fmatthew5876 Would it make sense to close this PR in favor of #2381? Because you have already added the priority weight fixes in your PR and you are planning to implement the remaining AI fixes.

@mateofio
Copy link
Contributor

mateofio commented Oct 9, 2020

Yes I think so, unfortunately

@mateofio mateofio closed this Oct 9, 2020
@ghost ghost deleted the issue-1831 branch October 10, 2020 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Battle Has PR Dependencies This PR depends on another PR
Development

Successfully merging this pull request may close these issues.

Verify correctness of Game_Enemy::ChooseRandomAction
2 participants