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

Fix GuiTraceRay for radardrift #1764

Merged

Conversation

saurtron
Copy link
Collaborator

@saurtron saurtron commented Nov 9, 2024

Work done

  • Introduce new quadField::GetQuadsOnWideRay.
  • Make TraceRay::GuiTraceRay use it for cases where useRadar=true.
  • Fixes not being able to select certain radar dots with direct click (those who drifted into different quads because of radar).

Related issues

Details

  • Does make the engine scan more quads, although most of the time still less than before Should not GuiTraceRay for quads beyond the floor until the far plane. #1754 when that fix is applied. (fix not applied here will need to be applied after/when that is merged)
  • I'm torn about enlarging the search area so much, but seems safest, even though I've generally seen the problem arise just close to the ground, since that's where generally targets are.
    • I enlarge the ray both forward and also to the sides.
  • Could be optimized making the area smaller but probably prone to error in corner cases:
    • Don't enlarge unless close to ground/ray intersection (should be quite safe, but might result in some error corner cases). Might be worth it imo.
  • Another approach could be adding radar dots to quads themselves, but each team having different radar positions could make this more cumbersome, also annoyingly a radar dot could also go into several quads if at the wrong place.
  • I tried to change as small part as possible from GetQuadsOnRay so changes can be reapplied, if that method needs to be modified at some point this one should follow suit.

Before

(images showing traced quads and the offset from radar position to real position for units in such quads)

note the dots inside the quads with no offset. those are the "error dots". they are coming from a different quad outside of the one they currently flying above, so they wouldn't be selectable.

wideray-pre2

After

hopefully no dots without selection, but a lot more quads searched. (note it's not exactly the same ray, so not directly comparable with the prev image, just to show no "error dots" are happening). generally with the solution we will be scanning 3x the quads since we need to scan around the usual set of quads to find possibly far away radar dots.

wideray

Diagram

Didn't color all quads properly but it should show the idea

ray-diagram

@sprunk
Copy link
Collaborator

sprunk commented Nov 15, 2024

Is the white quad debug view some standard /debugBla?

@saurtron
Copy link
Collaborator Author

saurtron commented Nov 15, 2024

Is the white quad debug view some standard /debugBla?

It's something quick and dirty I had to add myself to debug the ray and quad issues here, same with radardot-realpos line, and selected units quads. Was thinking about adding it somehow, but the fact it would require a check every quad at quad iteration inside GuiTraceRay, kind of puts me off.

Copy link
Collaborator

@sprunk sprunk left a comment

Choose a reason for hiding this comment

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

Looks good.

@lhog
Copy link
Collaborator

lhog commented Nov 24, 2024

Needs a rebase now

@saurtron
Copy link
Collaborator Author

Needs a rebase now

Merged, not sure if you prefer rebase. It shouldn't matter for this but let me know if you want me to change the procedure, this way it's also easier to see what changes I did for the merge.

@lhog
Copy link
Collaborator

lhog commented Nov 29, 2024

I have hard time reading GetQuadsOnWideRay (though not harder than reading the original GetQuadsOnRay).
I trust you have tested it thoroughly enough, haven't you?

@lhog
Copy link
Collaborator

lhog commented Nov 29, 2024

Also seems like I can't merge it now due to some sort of conflict anyway.

@lhog lhog merged commit 35c914e into beyond-all-reason:master Dec 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Radar dots cannot to be targeted sometimes
3 participants