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

Use getPosASLVisual for icons and camera position #5067

Merged
merged 2 commits into from
Jun 3, 2017

Conversation

rebelvg
Copy link
Contributor

@rebelvg rebelvg commented Apr 9, 2017

When merged this pull request will:

  • Fix stuttering of the camera and icons when camera is following the unit.
  • Moves icons directly to the unit position.

It uses getPosASLVisual and ASLToAGL which simulates what modelToWorldVisual does but it updates properly. Tested this in the spectator we use in my community, works properly for any surface (water, underwater, above ground).

Example video.
Old - https://youtu.be/j7L0hg4POtY?t=0
New - https://youtu.be/j7L0hg4POtY?t=16

@jonpas jonpas added kind/enhancement Release Notes: **IMPROVED:** status/review-pending labels Apr 9, 2017
@jonpas jonpas added this to the 3.10.0 milestone Apr 9, 2017
@commy2
Copy link
Contributor

commy2 commented Apr 9, 2017

So you're saying that modelToWorldVisual is b0rked or used wrong?

@rebelvg
Copy link
Contributor Author

rebelvg commented Apr 9, 2017

Some commands can't be used in per frame since BI fixed "teleporting" bug by introducing interpolation to the network code long time ago in arma 2 ao, that's why they came up with these getPosVisual and visiblePosition commands, they work properly if you want to do some per frame visual stuff.

@commy2
Copy link
Contributor

commy2 commented Apr 9, 2017

And why doesn't it work for modelToWorldVisual? Broken?

@rebelvg
Copy link
Contributor Author

rebelvg commented Apr 9, 2017

No idea.

@commy2
Copy link
Contributor

commy2 commented Apr 10, 2017

_unit modelToWorldVisual [0,0,0]

AGL from model center, render scope

ASLToAGL getPosASLVisual

AGL from lowest road contact point, render scope

I don't think these are identical. What would be needed is a getPosWorldVisual, but that does not exist. Manually add the distance from model center to lowest land contact point? That could be determined once via getPosASL select 2 - getPosWorld select 2.
Refresher: getPosWorld reports PosASL from model center.

@rebelvg
Copy link
Contributor Author

rebelvg commented Apr 25, 2017

I'm not sure what I'm doing wrong but they report 0 difference for ground units and flying units.
http://i.imgur.com/Hi37S7o.jpg

@kymckay
Copy link
Member

kymckay commented May 12, 2017

I admit I'm not currently familiar with the intricacies of this PR, but based on the video alone it seems like a good fix. However, it's also worth noting I'm currently undertaking a fairly significant refactor/overhaul of the spectator code based on BIs end game spectator code (which has already enlightened me to some things that can definitely be handled better - I suspect their method of icon rendering should also prove foolproof).

As for moving the icon to the exact position of the unit, personally I preferred it above the unit so that it isn't obscuring anything, but if this is preferable to most people I see no reason not to make the change.

@kymckay
Copy link
Member

kymckay commented May 19, 2017

This PR will be made redundant by #5171 (which I expect to have ready to merge fairly soon). However, it could be merged in as a temporary fix if a release occurs before that PR is merged.

Copy link
Member

@kymckay kymckay left a comment

Choose a reason for hiding this comment

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

This works as a temporary fix until the overhaul is complete. I back-ported the new unit icons too 😃

@kymckay kymckay merged commit bf4b49a into acemod:master Jun 3, 2017
@jonpas jonpas changed the title Use getPosASLVisual for icons and camera position. Use getPosASLVisual for icons and camera position Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants