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

Make QRFs and singleAttacks behave semi-reasonably with maxUnits #2113

Conversation

jaj22
Copy link

@jaj22 jaj22 commented Aug 31, 2021

What type of PR is this.

  1. Bug
  2. Change
  3. Enhancement

What have you changed and why?

Support QRFs and singleAttack (marker recapture counterattack) were using a maxUnits check (through chooseAttackType) that didn't account for unit locality. In Antistasi, maxUnits applies per machine rather than globally, so the permitted unit count scales up with the number of headless clients. As a result, QRFs and singleAttacks will have been much rarer or smaller than intended when headless clients were used.

This PR fixes the maxUnits checks so that they ignore units on other machines. A buffer is left for QRFs so that wavedCA doesn't get completely crippled by running on the same machine.

Please specify which Issue this PR Resolves.

closes #2098

Please verify the following and ensure all checks are completed.

  1. Have you loaded the mission in LAN host?
  2. Have you loaded the mission on a dedicated server?

Is further testing or are further changes required?

  1. No
  2. Yes (Please provide further detail below.)

maxUnits behaviour is generally terrible but that's a bigger problem to be addressed later.

More problematic is that singleAttacks and QRFs likely didn't spawn at all on the community servers, so that would invalidate any balance data. Might need to buff up economicsAI again, for example, and the upper-end vehicle counts in singleAttacks and QRFs are likely far too high, as that end hasn't been tested in an appropriate case.

How can the changes be tested?

WIP

@jaj22 jaj22 added Bug-fix Change A change to existing functionality Balance For issues and PRs regarding game balance. Don't merge This needs to be touched before being merged labels Aug 31, 2021
@jaj22
Copy link
Author

jaj22 commented Aug 31, 2021

Had a better idea. Make maxUnits a global value and don't attempt to micro-manage individual machines. Should make things more stable and transparent.

@HakonRydland
Copy link
Collaborator

Had a better idea. Make maxUnits a global value and don't attempt to micro-manage individual machines. Should make things more stable and transparent.

couldnt that cause load issues where say one machine could have most of the units on it and another having like 10 units?

@jaj22
Copy link
Author

jaj22 commented Aug 31, 2021

couldnt that cause load issues where say one machine could have most of the units on it and another having like 10 units?

Routines that use the HCs (garrisons, missions, singleAttack, reinfPatrols) are sent to the one with the lowest unit count anyway. Major attacks and supports (including post-2.4 QRFs) run on the server, but as there's only one attack permitted at once, and QRFs are limited by other means (this bit is questionable), I don't think there's much risk of heavy asymmetry. The worst case is already happening: Single HC gets overloaded with garrisons and singleAttacks while the server is basically idle.

@jaj22
Copy link
Author

jaj22 commented Aug 31, 2021

On second thoughts, the worst part is that garrisons ignore the limit but count towards it, potentially blocking out everything else. This case is potentially worse with a global limit because at least garrisons don't spawn on the server when you have HCs, so QRFs and major attacks should still run.

Maybe leave this as-is and rethink the whole system later. Might throw in another PR to sort out some particularly spammy garrisons though.

@jaj22 jaj22 removed the Don't merge This needs to be touched before being merged label Aug 31, 2021
@jaj22 jaj22 added this to the 2.5.3 milestone Sep 1, 2021
@Bob-Murphy Bob-Murphy merged commit da0f4ad into official-antistasi-community:unstable Sep 20, 2021
@Bob-Murphy Bob-Murphy added the Added to changelog Added to changelog label Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added to changelog Added to changelog Balance For issues and PRs regarding game balance. Change A change to existing functionality Ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: QRFs don't check locality with maxUnits
3 participants