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

Change MP Initial Player Wing Ship Behavior to Match SP #2593

Merged

Conversation

JohnAFernandez
Copy link
Contributor

@JohnAFernandez JohnAFernandez commented Jul 24, 2020

The way that this function was structured before would 1. Change the AI mode of player ships (very minor) 2. Keep AI in player wings from doing anything because they were trying to form on the wing of the standalone ship placeholder.

To clarify, this doesn't mean that they will now start out fighting without order. Instead, they should start out only forming on the wing of their team leader.

Need to do some more testing to make sure that this is 100% fixed, since Alpha 4 was acting strange during the last test, but otherwise, the initial test looked good.

Mainly just need to test Team vs. Team on standalone. This is apparently impossible on one computer, as I just tried and both clients hit an assert.

@JohnAFernandez
Copy link
Contributor Author

And here's the related Mantis issue. http://scp.indiegames.us/mantis/view.php?id=1815

Hopefully a complete fix for the issue.

@JohnAFernandez JohnAFernandez added fix A fix for bugs, not-a-bugs, and/or regressions. mantis A feature or issue imported from MANTIS, the old issue tracker multi A feature or issue related to the multiplayer code. labels Jul 24, 2020
Copy link
Contributor

@Goober5000 Goober5000 left a comment

Choose a reason for hiding this comment

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

Left some notes.


aip = &Ai_info[Ships[wingp->ship_index[j]].ai_index];
// don't process Player_ship
if ( aip == Player_ai )
if (aip == Player_ai || (MULTIPLAYER_MASTER && Objects[Ships[aip->shipnum].objnum].flags[Object::Object_Flags::Player_ship])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the MULTIPLAYER_MASTER check need to be here? A player ship shouldn't be processed regardless of what type of build is running.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check for multiplayer guarantees two things: one, that the game is in multiplayer so SP will not be affected, and two, that only the server will make these changes. Since the server takes care of the all of the ai (it gets sent via object update packets to the client), doing otherwise could cause server and client to go out of sync temporarily, albeit probably not noticeably.

Copy link
Contributor

Choose a reason for hiding this comment

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

If only the server should make these changes, then shouldn't you instead put

if (multiplayer && !MULTIPLAYER_MASTER)
    return;

at the very top of the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmmm.... Yes, that makes sense. Perhaps in the future someone can go through and further optimize the ai code for clients for similar functions.

else {
ai_add_ship_goal_player(AIG_TYPE_PLAYER_SHIP, AI_GOAL_FORM_ON_WING, -1, Ships[Wings[Starting_wings[0]].special_ship].ship_name, aip);
}
} // need to add a form on my wing goal here. Ships are always forming on the player's wing.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment got misplaced. Put it above the "it is sufficient" line because it describes what the entire if() block is doing.

Comment on lines 165 to 168
ai_add_ship_goal_player(AIG_TYPE_PLAYER_SHIP, AI_GOAL_FORM_ON_WING, -1, Ships[Wings[TVT_wings[ship_regp->p_objp->team]].special_ship].ship_name, aip);
}
else {
ai_add_ship_goal_player(AIG_TYPE_PLAYER_SHIP, AI_GOAL_FORM_ON_WING, -1, Ships[Wings[Starting_wings[0]].special_ship].ship_name, aip);
Copy link
Contributor

Choose a reason for hiding this comment

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

The special_ship field refers to the position in the wing, not the ship's shipnum. But regardless, I am about to remove the special_ship field, because it has a fundamental design flaw. Instead, have the ships form on the wing's ship_index[0]:

Ships[Wings[TVT_wings[ship_regp->p_objp->team]].ship_index[0]].ship_name

Ships[Wings[Starting_wings[0]].ship_index[0]].ship_name

Also, as an edge case, if for some reason the wing leader is AI-controlled, it will be given an order to form on itself. That will need to be checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

Copy link
Contributor

Choose a reason for hiding this comment

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

As it turns out, special_ship might be fixable, so do this:

int wingnum = TVT_wings[ship_regp->p_objp->team];
...Ships[Wings[wingnum].ship_index[Wings[wingnum].special_ship]].ship_name...

int wingnum = Starting_wings[0];
...Ships[Wings[wingnum].ship_index[Wings[wingnum].special_ship]].ship_name...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right. Thanks for putting that together!

@@ -150,25 +156,26 @@ void ai_maybe_add_form_goal(wing* wingp)

aip = &Ai_info[Ships[wingp->ship_index[j]].ai_index];
// don't process Player_ship
if (aip == Player_ai || (MULTIPLAYER_MASTER && Objects[Ships[aip->shipnum].objnum].flags[Object::Object_Flags::Player_ship])) {
if (aip == Player_ai || ((Game_mode & GM_MULTIPLAYER) && Objects[Ships[aip->shipnum].objnum].flags[Object::Object_Flags::Player_ship])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't even think you need the multiplayer check at this line now. In single-player, only the player ship is the player ship.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So just if (Objects[Ships[aip->shipnum].objnum].flags[Object::Object_Flags::Player_ship]) to check if it is a player ship?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right.

@JohnAFernandez
Copy link
Contributor Author

Ok, that should take of the feedback. Once we have had the ability to test this in a team v. team on standalone setting to make sure the behavior is correct, I will switch this to ready for review.

@JohnAFernandez
Copy link
Contributor Author

Team v Team has been tested and works fine.

@JohnAFernandez JohnAFernandez marked this pull request as ready for review August 30, 2020 05:34
@Goober5000 Goober5000 merged commit a710aac into scp-fs2open:master Aug 31, 2020
@JohnAFernandez JohnAFernandez deleted the Fix-Broken-Could-Be-Player_AI branch October 9, 2022 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for bugs, not-a-bugs, and/or regressions. mantis A feature or issue imported from MANTIS, the old issue tracker multi A feature or issue related to the multiplayer code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants