TradeShips hyperjump code change

[Max5377]

This PR is an attempt to fix cluttering hyperclouds around stations and rare illegal jumps almost at the planet's surface. Now TradeShips behaviour when undocking looks like this.
For orbital stations:

1. Undock;
2. Fly to limits of current orbital station;
3. OnAICompleted event fires, tell ship to fly to current system's star;
4. Assign task to check distance between current orbital station and ship, if distance is large enough, make the jump.

For ground stations;

1. Undock;
2. Tell ship to fly to current system's star;
3. Assign task to check altitude between current orbital station's parent and ship, if altitude is high enough, make the jump.

The distance and altitude will be selected from a random interval of 20,000 - 240,000 meters.
Demonstration:

1. Undocking and jumping from Cydonia

Hyperjump_Ground.mp4

2. Undocking and jumping from Gates Starport

Hyperjump_Orbital.mp4

Update: rebase.
Update2: addEquip is removed from setPlayerAsTraderDocked and setPlayerAsTraderInbound.
Added check if player is currently docked (setPlayerAsTraderDocked) or there's nearest station (setPlayerAsTraderInbound).
If not, player will not become trader.
Update3: rebase.

[Max5377]

There's still a possibility for this(ex. for Andersen City in Ahonen's Claim in sector Lalande 21185):
Far view:

Station view:

But in my opinion it doesn't look this bad as showed here:
#5539 (comment)
#5539 (comment)
Edit:
There's also a possibility of illegal hyperjumps, because in the Ship.IsHyperjumpAllowed GetGroundPosition is used and it has this check:

if (!f->IsRotFrame())
    return 0;

For example, for moon this will be true at ~6000m, which is still illegal for making the jump. I addressed this in the PR.
No concern with this function for TradeShips with this change, but it will be true if this is called not from here (some mission modules use it).

[Max5377]

@Gliese852

1. There's a possibility of inbound ship coliding with planet's terrain, exploding in the process. Maybe it's better to tell ship to orbit planet first and only then dock with station?
   (autopilot also has this problem);
2. There is still a possibility of undocking ship breaking the rules and making illegal hyperjump when it's under attack
   (Perhaps can be explained that pilot wants to save his live, not caring about rules at the moment);
3. You were right. Ships are jumping at higher distances when game speed is 10000x. I also discovered this behaviour in the original TradeShips code too
   (Can't think of any solution for this, unfortenately, unless it's not big of a deal);
4. The second step of behaviour in orbital stations would be skipped if there wasn't a problem with pathfinding when ship is inside orbital station
   (They basically will ignore walls, start to collide with them and possibly exploding in the process);
5. I included setPlayerAsTraderDocked and setPlayerAsTraderInbound in Flow part of TradeShips so you can view TradeShips AI from player's eyes (used in the demonstration)
   (Let me know if you want to move it from here or delete it completely);
6. Also, right now in my changed code I'm using GetAltitudeRelTo, which has a problem of using player pos in calculating altitude, skipping first argument, but I've done PR that I mentioned above.
   Aside from this I haven't discovered any problems.

[Max5377]

Forgot to mention that I also had rare case with old code, where when I was jumping to Bernard's Star from Sol and back and going to the Moon after that, I would see hyperspace cloud below planet's terrain. I have a save to demonstrate what I'm talking about:
CloudBelowTerrain.zip

Haven't seen this with my change, but who knows. I hope this will fix this.

[Gliese852]

@Max5377

1. There's a possibility of inbound ship coliding with planet's terrain, exploding in the process. Maybe it's better to tell ship to orbit planet first and only then dock with station?
   (autopilot also has this problem);

Ideally, of course, it would be worth solving this problem inside the autopilot, but if such a workaround will reduce the number of victims, I’m for it.

2. There is still a possibility of undocking ship breaking the rules and making illegal hyperjump when it's under attack
   (Perhaps can be explained that pilot wants to save his live, not caring about rules at the moment);

I think this is quite correct behavior.

3. You were right. Ships are jumping at higher distances when game speed is 10000x. I also discovered this behaviour in the original TradeShips code too
   (Can't think of any solution for this, unfortenately, unless it's not big of a deal);

I'm inclined to think that this is not big of a deal.

4. The second step of behaviour in orbital stations would be skipped if there wasn't a problem with pathfinding when ship is inside orbital station
   (They basically will ignore walls, start to collide with them and possibly exploding in the process);

It’s strange, it works the same for me, besides, both commands ultimately come down to calling AICmdFlyTo, where it turns out that the body is too close to the center of the orbital and the ship tends to move away normally. Maybe you could provide a save file where the difference in behavior is reproduced?

5. I included setPlayerAsTraderDocked and setPlayerAsTraderInbound in Flow part of TradeShips so you can view TradeShips AI from player's eyes (used in the demonstration)
   (Let me know if you want to move it from here or delete it completely);

There are some problems with these functions, as seen in the second video, Probably, you shouldn’t try to Trader.addEquip, because it falls with assertion. Overall this seems like it might be quite useful. I would also like to note that undocking on the autopilot already works for the player, so you can give the autopilot a command while on the pad and see what happens.

[Max5377]

@Gliese852

Ideally, of course, it would be worth solving this problem inside the autopilot, but if such a workaround will reduce the number of victims, I’m for it.

Didn't do anything really. The death rate for Sol system is ~50+/-10 ships per week no matter what approach is used.
Also wanted to add that ships are sometimes colliding with each other too (inbound and outbound) and looks like it usually happenes at orbital stations (according to TradeShips log).
Let Lalande 21185 be a stress-test for fixing ships AI, because the death rate for this system is insane (179 ships in a week first try) and for some reason in this system there's a lot of deaths from the star itself (according to TradeShips log too).

It’s strange, it works the same for me, besides, both commands ultimately come down to calling AICmdFlyTo, where it turns out that the body is too close to the center of the orbital and the ship tends to move away normally. Maybe you could provide a save file where the difference in behavior is reproduced?

My bad. I'll try to show and explain what happenes.

Giving ship command to fly to body, when ship is more to the center of orbital starport, the more chance it will deviate from the course and fly into the wall instead, especially if it rotates backwards beforehand.
Here's video to demonstrate this:

AIFlyTo_Near_The_Center.mp4

If ship is in the center of orbital station, he will usually fly upwards and also try to go through wall, no matter if it's moving or not.
Demonstration:

AIFlyTo_Center.mp4

This happenes with all commands that tell ship to fly to something (AIFlyTo, AIEnterLow/Medium/HighOrbit).
You can also experience it by going into big stations (Jobs Pad, Gates Spaceport) and setting maximum speed. Soon you will hear collision sounds. I will give you the save, but...

I spotted another issue:

1. Start new game;
2. Wait around for some time;
3. Save;
4. Load;
5. Wait around for some time.

There's chance of game failing with this:

After diging around more, this happenes at this line in Events.lua in onShipDocked:

local damage = ShipDef[trader.ship_name].hullMass - ship.hullMassLeft
if damage > 0 then
	ship:SetHullPercent()
	Trader.addEquip(ship) <--
end

This behaviour is also reproducable in the original TradeShips code.

Ship's behaviour on Orbital stations.zip

Updated saves for current master:
Ship's behaviour on Orbital stations (current master).zip

There are some problems with these functions, as seen in the second video, Probably, you shouldn’t try to Trader.addEquip, because it falls with assertion. Overall this seems like it might be quite useful. I would also like to note that undocking on the autopilot already works for the player, so you can give the autopilot a command while on the pad and see what happens.

Yes. It is enough to order the ship to undock after making the player a trader. No point in exposing trader_task table in Trader.lua (I didn't include this anyway, but showed it in the demonstration). I will also delete addEquip. And I don't know about Space.PutShipOnRoute. It frequently puts my ship in what looks like the center of the ground station's model, unless I haven't forgot something (or it's intended).
Demonstration (I set Engine.rand:Number(0.0, 0.999) to Engine.rand:Number(0.999, 0.999) beforehand):
For ground station:

PutShipAtPointOfStation_Ground.mp4

For orbital station:

PutShipAtPointOfStation_Orbital.mp4

[Max5377]

For the assertion issue:
TradeShips is not causing it. Explanation.

About Space.PutShipOnRoute:
Did a quick test for this. Started new game, spawned docked ship and entered this in the console:
require("Space").PutShipOnRoute(require("Game").player, require("Game").player:FindNearestTo("SPACESTATION"), 0.999)

Then clicked on said ship and entered this:
require("Space").PutShipOnRoute(require("Game").player:GetCombatTarget(), require("Game").player:GetCombatTarget():FindNearestTo("SPACESTATION"), 0.999)

The next I see that this ship is moved inside my ship.

Then I wanted to see what will happen if I move my ship to distant station. I set Gates Spaceport as my navTarget and entered this:
require("Space").PutShipOnRoute(require("Game").player, require("Game").player:GetNavTarget(), 0.999)

My ship just collided with said station on high speed..

I tried to test this on ground station, my ship moved close to it, but it also had high speed that I could never stop with my current fuel reserves that got substracted.
I don't think I will undestand something by looking at code for this function, because I haven't worked in this field before (trigonometry and similar stuff).

[Gliese852]

@Max5377 have you read the description of PutShipOnRoute? I would like to point out that if the distance to the target is short enough, a t_ratio of 0.999 will place you almost at the target coordinates.
The only purpose of this function is to arrange the tradeships in the system when the player enters the star system (or starts the new game) as if they had been flying here for a long time already.

This happenes with all commands that tell ship to fly to something (AIFlyTo, AIEnterLow/Medium/HighOrbit).

Yes, because, as I already wrote, the first thing a ship does is try to get out of the "object’s radius" along the normal, that is, along the ray connecting the center of the station and the center of the ship. So, if the ship is lucky and it is exactly on the axis of the station, it will fly through the gate, if it is shifted off the axis, it will crash somewhere.

[Max5377]

@Gliese852 Sorry, math is just not my best subject, really. But, I kinda understand what you mean now. Thanks for clarification.
Do you have any suggestions about TradeShips code itself or all good?

[Gliese852]

@Max5377 Looks like it's still a draft? I see blots in the code (occasionally spaces/tabs at the end of the line), commented out pieces.

[Max5377]

@Gliese852 I deleted parts of commented old code that doesn't needed anymore.
Apart from the issue that I mentioned here about GetAltitudeRelTo, it should work without any other issues.

[impaktor]

@Max5377 Whatever editor you're using, I'm sure it should be trivial to have it nuke trailing white spaces on save. Also to respect tabs/spaces. We have an editorconfig-file

[Max5377]

@Gliese852 Also pushed fix for lua error when you try to hyperspace with TradeShips debug tab opened.

[Web-eWorks]

I'll generally leave the approval and merge duties to @Gliese852 as this is code he's most familiar with. I apologize for the long delay in contributing my review of this code - I've only noticed one issue that I'd appreciate being addressed before merge, but I otherwise would like to merge this in the next few weeks if there are no outstanding objections.

Thank you for addressing these bugs!

[Gliese852]

@Max5377 I think it’s worth doing a rebase again, in the current state I always have a crash at the beginning of the hyperjump.

[Max5377]

@Gliese852 Did rebase to current master. Shouldn't crash now.