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.

Sign up for GitHub

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

Jump to bottom

Consider AutoTarget ScanRadius when attack moving #20257

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Consider AutoTarget ScanRadius when attack moving #20257

merged 1 commit into from
Jul 25, 2023

Conversation

PunkPun
Copy link
Member

@PunkPun PunkPun commented Sep 4, 2022 •
edited
Loading

closes #17503

After 4 years melee units will be able to attack move again

This PR basically reverts #16942 for units on AttackAnything stance

@PunkPun PunkPun added this to the Next release milestone Sep 4, 2022
@Mailaender
Copy link
Member

I wouldn't call it a revert, you made it configurable which looks like the right way to go.

Mailaender
Mailaender previously approved these changes Sep 9, 2022
View reviewed changes
@Mailaender Mailaender requested review from Mailaender and removed request for Mailaender September 12, 2022 18:26
Mailaender
Mailaender reviewed Sep 12, 2022
View reviewed changes
Copy link
Member

@Mailaender Mailaender left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I misunderstood the change and thought this would solve the problem by moving it to a per-unit setting in .yaml

@Mailaender Mailaender removed this from the Next release milestone Sep 12, 2022
@PunkPun PunkPun added this to the Next release milestone Sep 12, 2022
@PunkPun
Copy link
Member Author

PunkPun commented Sep 13, 2022

Screenshot 2022-09-13 at 13 25 09

@PunkPun
Copy link
Member Author

PunkPun commented Sep 13, 2022 •
edited
Loading

The first part is crucial for melee units, as they fully rely on ScanRadius.

The second part is there to keep the behaviour consistent and not introduce any code landmines for the future from the behaviour mismatch

@abcdefg30 abcdefg30 modified the milestones: Next release, Next + 1 Dec 1, 2022
@PunkPun
Copy link
Member Author

PunkPun commented Mar 15, 2023

Testcase in RA - spies. They have a long autoscan radius

@penev92
Copy link
Member

penev92 commented Jul 25, 2023

@abcdefg30 I seem to remember you were not convinced by something here?

@abcdefg30
Copy link
Member

I wasn't convinced about the attack-move behaviour, but I think this PR is fine for now. (The problem is units endlessly chasing instead of moving to their attack-move target. But that is OK in this PR since it only applies to units in the AttackAnything stance.) Hence I removed the label. :)

penev92
penev92 approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

@penev92 penev92 left a comment

Choose a reason for hiding this comment

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

OK. Hopefully this will get thoroughly tested during playtesting.

@Mailaender Mailaender mentioned this pull request Jul 25, 2023
Closed
@Mailaender Mailaender merged commit 3207d01 into OpenRA:bleed Jul 25, 2023
3 checks passed
@Mailaender
Copy link
Member

Changelog

@PunkPun PunkPun deleted the fix-melee branch July 25, 2023 18:09
@PunkPun PunkPun mentioned this pull request Jul 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mailaender Mailaender Mailaender left review comments

@penev92 penev92 penev92 approved these changes

Assignees
No one assigned
Labels
PR: Needs +2
Projects
None yet
Milestone
Release 20231010
Development

Successfully merging this pull request may close these issues.

On attack move auto-target scan radius beyond weapon range seems to be fully ignored
4 participants
@PunkPun @Mailaender @penev92 @abcdefg30