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 Reinforcements.ReinforceWithTransport for planes #12813

Closed
wants to merge 2 commits into
base: bleed
from

Conversation

Projects
None yet
6 participants
@abcdefg30
Member

abcdefg30 commented Feb 20, 2017

Changes
image
to
image

@pchote

This comment has been minimized.

Member

pchote commented Feb 20, 2017

Isn't this kind of bogus? Aircraft aren't supposed to be able to land on the ground.

@GraionDilach

This comment has been minimized.

Contributor

GraionDilach commented Feb 20, 2017

Mission aircraft could be. This looks like something allowing to simulate ProductionAirdrop to me without a dummy producer actor.

@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented Feb 20, 2017

Isn't this kind of bogus? Aircraft aren't supposed to be able to land on the ground.

Checks for LandableTerrainTypes could be added, but I think that's an unnecessary restriction/ complication. You can already create actors at "bogus" (unpathable) locations (e.g. rilfe men on water tiles). That is simply a scripting error then.

@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented Feb 20, 2017

I'm also not sure if the production airdrop mentioned by @GraionDilach is possible when adding checks for LandableTerrainTypes. We'll at least need that for some d2k missions.

@reaperrr

This comment has been minimized.

Contributor

reaperrr commented Mar 19, 2017

Isn't this kind of bogus? Aircraft aren't supposed to be able to land on the ground.

Maybe not in our mods, but at least for 3rd-party mods (any maybe some missions) this is a sensible feature, in my opinion.

@obrakmann

Looks otherwise ok

Move(transport, entryPath[i]);
var lastPosition = entryPath.Last();

This comment has been minimized.

@obrakmann

obrakmann Apr 17, 2017

Contributor

Can you rename this destination or dropoffPoint or something? lastPosition is confusing.

var delta = transport.CenterPosition - transport.World.Map.CenterOfCell(lastPosition);
var sign = delta.LengthSquared >= landDistance * landDistance ? 1 : -1;
var positionBeforeLanding = lastPosition + sign * new CVec((int)horizontal, (int)vertical);

This comment has been minimized.

@obrakmann

obrakmann Apr 17, 2017

Contributor

Something must be wrong with this. It causes the noticeable drop right before touchdown. If I add 1024 to the landDistance, the landing is smooth as expected, but I'm not sure if that's actually the part where the error is, and I forgot too much of my highschool math to understand what's happening here.

@pchote

This comment has been minimized.

Member

pchote commented May 13, 2017

@abcdefg30: Are you planning to fix this, or shall we close it?

@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented May 14, 2017

I plan to look into this the coming week.

@reaperrr

This comment has been minimized.

Contributor

reaperrr commented Jun 23, 2017

I plan to look into this the coming week.

That obviously didn't work out, will you resume work on it soon or can this be closed for now?

@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented Jul 15, 2017

Sorry for the very late update. @obrakmann: fixed those issues. I rewrote quite a bit of the original code and added some comments, so I hope it easier to understand now.

if (actionFunc != null)
{
Move(transport, dropoffPoint);

This comment has been minimized.

@pchote

pchote Jul 22, 2017

Member

If entryPath has one element then this will queue a Move to it that we don't want (it is excluded in the old code by starting the loop at index 1).

var dropoffCenter = map.CenterOfCell(dropoffPoint);
var speed = aircraft.Info.Speed;
// Distance from the altitude to the ground

This comment has been minimized.

@pchote

pchote Jul 22, 2017

Member

🚨 DUPLICATION ALERT 🚨

Please adapt ReturnToBase / Land logic to cover your use case, don't just copy half of its code here!
Really, this linking up logic should be part of Land itself...

@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented Aug 8, 2017

This'll want to wait for #13785.

@pchote

This comment has been minimized.

Member

pchote commented Oct 21, 2017

Closing stale PR. You know the boilerplate about reopening when ready by now 😄

@pchote pchote closed this Oct 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment