Improve tradeships #4984

Gliese852 · 2020-10-26T18:30:00Z

tradesh_git.mp4

TODO

create a pre-calculation of the ship's path according to the specified parameters
create a function that spawns ships "on the way" with checking for possible collisions at the insertion point
improve the method of creating merchant ships, using these functions (TradeShips.lua)

Commits

Add class for pre-calculating spaceship path

A one-dimensional case is considered, given the parameters of the ship
and the length of the path, a flight plan is calculated, which allows
determining the time, speed, and mass of the ship at any point on the
way. Also added a simple simulator for testing purposes.

Add function PutShipOnRoute

Function description:

The ship rearranged from its current position to a given body in space, for a given part of the path, as if it were flying in a straight line, consuming fuel and changing speed. The ship's current speed is ignored and is considered to be equal to target's.
This function changes the coordinates, the mass of fuel in the tank, and the speed of the given ship.

This function will be used so that when the game starts, or the player jump into the system, the NPC ships looked as if they had been moving on their routes for a long time.

Improve Tradeships.lua

Preparations:

Add method Ship:GetCargo
Add method Ship:GetDurationForDistance
Add method Ship:Create
Add method ui.treeNode & ui.treePop
Add method LuaObject::CreateInLua
Add functions for convenient selection of a random array element
Add some numerical utilites
Add a tool to easily draw a table from an array
Fix ui.Format.Number
Turn off the output when the ship changes frames
Reduce the detection range of hyperspace clouds
Remove the call LuaEvent::Emit immediately after exiting hyperspace
Split Tradeships.lua into 5 modules

Tradeships.lua

For current system, calculate all possible routes of tradeships, durations, flows; arrange ships in space in such equilibrium state, as if the have been flying about their business for a long time.
Calculate the average spawn interval for new ships, and parking intervals, so that the stations are not overcrowded or empty.
Сreate a tab in the debug window with full information on routes, ships, stations, remote systems.

Fix some bugs appeared

Move the drawing of the system map pigui to a separate handler to ensure that it will always be drawn strictly after the SystemView object.
Fix the setting of the m_colliding parameter of the ship during deserialization.

Reduce the flow of tradeships

reduce the flow to the most popular station in system by 40%.
reduce the maximum station load by 25%
reduce the maximum ships limit by 40%

This is to improve the performance. although these parameters are independent, it is better to change them synchronously. For example, if you just reduce the allowable number of ships, in all systems this number can become the same. If the load on the station is not reduced, the ships will stay there for too long.

Details on the forum: https://pioneerspacesim.net/forum/viewtopic.php?f=3&t=650

The-EG · 2020-10-26T19:52:05Z

Just a heads up: adding sources below src/ automatically adds them to the build, which I'm guessing you didn't intend for src/ship/PPTester since it has a main. Also, it looks like you're missing #include <algorithm> for std::min.

Gliese852 · 2020-10-26T20:01:16Z

@The-EG Thanks! To be honest, I have not yet figured out what to do with PPTester.cpp, probably this is a draft for a unit test, if I figure out how to make it properly. Or just throw it away.

impaktor · 2020-11-04T08:29:03Z

@Gliese852 Perhaps it's something we can add to the Ctrl-i menu? (I don't know what it does, but I like in-game debug tools!)

Gliese852 · 2020-11-04T18:42:23Z

@impaktor I just can't think of how it can be useful in debugging. Rather, it can be useful for navigation in space. With minor modifications it will be possible to calculate the time of arrival at the station, for example.

impaktor · 2020-11-22T09:00:33Z

Yay! The two new Github Actions ran (gcc & clang).

Gliese852 · 2020-11-23T09:01:06Z

@impaktor it's strange that the checks passed, locally I get a linking error, because there are two main functions

The-EG · 2020-11-23T18:02:46Z

Odd that the checks aren't at least warning about the duplicate main symbols, but it's definitely there.
The appveyor build actually picked up the main defined in PPTester.cpp by the looks of it. That's supposed to be the modelcompiler...I'm guessing GCC and clang are doing something similar.

Gliese852 · 2021-01-04T08:10:11Z

I decided to replace a separate test file PPTester.cpp with the ability to output a test report directly inside the game.
if you define the value PRECALCPATH_TESTMODE, while calling the functions of this class, a report similar to this will be output:

--------------------------------------------------------------------------------
Testing PrecalcPath::setTime()
.
Ship parameters:
EV:             18200000.00
F:               6100000.00
acap:                   inf
mass:             188000.00
fuel:              48000.00
margin:                0.90
.
Path and point parameters:
Stotal:     139533955305.25
V0:                    0.00
time:              96876.48
.
parameter |      calculated |       simulated |   deviation | result
----------|-----------------|-----------------|-------------|-------
Duration  |       126804.42 |       126804.50 |    0.00006% | OK
Vmax      |      2201119.79 |      2201119.26 |    0.00002% | OK
V         |      1080400.80 |      1080399.30 |    0.00014% | OK
fuel      |        16635.80 |        16635.79 |    0.00004% | OK
dist      | 123206922481.00 | 123207023018.29 |    0.00008% | OK
state     |              -1 |              -1 |             | OK
.
Test passed successfully!
--------------------------------------------------------------------------------

Gliese852 · 2021-02-06T20:07:30Z

now it's even playable

sturnclaw · 2021-03-08T20:20:36Z

@Gliese852 given that the TradeShips.lua file is now well over 1800 lines of Lua code (even with debug code), I would highly recommend that you create a folder for it in data/modules/ and split the file up into several sub-files. It massively helps readability to have similar functionality grouped into smaller files, and also means we can eventually promote functionality that's common to multiple modules to a top-level library.

sturnclaw

A few things stood out to me; I'll wait to review the Lua until it's a little bit more manageable (and I have more time).

src/lua/LuaSpace.cpp

src/ship/PrecalcPath.cpp

Gliese852 · 2021-03-11T22:57:48Z

@Gliese852 given that the TradeShips.lua file is now well over 1800 lines of Lua code (even with debug code), I would highly recommend that you create a folder for it in data/modules/ and split the file up into several sub-files. It massively helps readability to have similar functionality grouped into smaller files, and also means we can eventually promote functionality that's common to multiple modules to a top-level library.

@Web-eWorks divided Tradeships.lua into 5 modules, and brought something to the libs

sturnclaw

Slowly but surely making my way through this review. Here's a batch of change requests; I'll dig into the meat of the PR further tomorrow.

data/libs/Ship.lua

data/libs/options.lua

data/libs/utils.lua

data/modules/TradeShips/Core.lua

data/modules/TradeShips/Debug.lua

sturnclaw

Yet more changes - I'm sure some of these are pointing at code you didn't write, but it's a good time to do some cleanup of the module anyways.

data/modules/TradeShips/Events.lua

data/modules/TradeShips/Trader.lua

sturnclaw

Yet another batch of changes; only three files are left to review but I'm out of time to review those right now.

data/pigui/baseui.lua

src/Body.cpp

src/Frame.h

src/Game.cpp

src/lua/LuaObject.h

src/lua/LuaShip.cpp

Gliese852 · 2021-04-10T21:39:07Z

@Web-eWorks probably I will put the changes that I make in separate commits, and then I will squash them all where necessary

src/ship/PrecalcPath.h

data/modules/TradeShips/Flow.lua

sturnclaw

I haven't had a chance to playtest this (or anything else) for a while now, but the code is good to go! Thank you @Gliese852 for working so hard to make the code easy to maintain and understand for contributors (and for putting up with my pedantic style suggestions 😄) - this PR is quite a big one, but you've done an excellent job with it.

Gliese852 · 2021-04-27T04:47:47Z

@Web-eWorks Thank you very much for taking so much of your time to review!
I will squash all tempourary commits soon.

I played for a few hours, mostly courier missions, everything seems to work stably.
Savegame size with 700 ships in space is about 500 kb.
FPS dropped significantly but your 'shipfest' branch will fix the problem.
If the FPS turns out to be uncomfortable afterwards, it is quite easy to reduce the flow of ships in the master.
There are some non-critical issues with autopilot, but fixes are almost ready in #5127

A one-dimensional case is considered, given the parameters of the ship and the length of the path, a flight plan is calculated, which allows determining the time, speed, and mass of the ship at any point on the way. Also added a simple simulator for testing purposes.

Function description: The ship rearranged from its current position to a given body in space, for a given part of the path, as if it were flying in a straight line, consuming fuel and changing speed. The ship's current speed is ignored and is considered to be equal to target's. This function changes the coordinates, the mass of fuel in the tank, and the speed of the given ship. This function will be used so that when the game starts, or the player jump into the system, the NPC ships looked as if they had been moving on their routes for a long time.

- Preparations: Add method Ship:GetCargo Add method Ship:GetDurationForDistance Add method Ship:Create Add method ui.treeNode & ui.treePop Add method LuaObject::CreateInLua Add functions for convenient selection of a random array element Add some numerical utilites Add a tool to easily draw a table from an array Fix ui.Format.Number Turn off the output when the ship changes frames Reduce the detection range of hyperspace clouds Remove the call LuaEvent::Emit immediately after exiting hyperspace Split Tradeships.lua into 5 modules - Tradeships For current system, calculate all possible routes of tradeships, durations, flows; arrange ships in space in such equilibrium state, as if the have been flying about their business for a long time. Calculate the average spawn interval for new ships, and parking intervals, so that the stations are not overcrowded or empty. Сreate a tab in the debug window with full information on routes, ships, stations, remote systems.

- Move the drawing of the system map pigui to a separate handler to ensure that it will always be drawn strictly after the SystemView object - Fix the setting of the m_colliding parameter of the ship during deserialization

Gliese852 · 2021-04-27T06:10:20Z

Squashed all tempourary commits

fluffyfreak · 2021-04-27T21:17:07Z

Don't worry about the VS failure, I'll fix that up once it's merged.

sturnclaw · 2021-05-02T00:52:16Z

Building in RelWithDebInfo mode, I'm noticing a major frame drop in Sol when turning ship orbit display on in the System View. While I'd like to further improve that particular view's CPU usage, for now I'd suggest dropping the number of tradeships down to ~300 (from 500 in Core.lua) in deference to the members of our community with lower-spec PCs. I'd love to make that number configurable (and be able to bring it back up) but right now I don't think this is quite performant enough to merge as the 'default'.

Otherwise, this is an amazing PR and I really love the way the tradeship debug view looks with the new debugger color scheme!

impaktor · 2021-05-02T12:25:31Z

What's RelWithDebInfo?

OK, so just drop default number of ships and this will be ready for merge?

sturnclaw · 2021-05-02T16:06:07Z

What's RelWithDebInfo?

OK, so just drop default number of ships and this will be ready for merge?

Release with Debug Info - it's a default Cmake build target that compiles with optimizations and debug symbols (what we ship in releases).

Yes, just drop the default number of ships until such a time as we're able to optimize some of the remaining bottlenecks in the code, and this PR is ready to merge.

- reduce the flow to the most popular station in system by 40%. - reduce the maximum station load by 25% - reduce the maximum ships limit by 40% This is to improve the performance. although these parameters are independent, it is better to change them synchronously. For example, if you just reduce the allowable number of ships, in all systems this number can become the same. If the load on the station is not reduced, the ships will stay there for too long.

Gliese852 · 2021-05-02T19:41:30Z

@Web-eWorks I'll probably make this change in a separate commit to clarify how the flow is set up.

sturnclaw · 2021-05-02T21:26:29Z

Merged! Thank you for all of the work on this @Gliese852!
@fluffyfreak whenever you've recovered, the VS files need updating for the new files involved in the PR.

Gliese852 force-pushed the tradeships branch from 5c406bc to 5a298cd Compare October 26, 2020 19:55

Gliese852 force-pushed the tradeships branch from 5a298cd to 79d9ffd Compare October 26, 2020 20:03

sturnclaw force-pushed the master branch from 3c682cf to 5443bb9 Compare November 20, 2020 23:17

Gliese852 force-pushed the tradeships branch 2 times, most recently from be227fa to f67523a Compare November 21, 2020 20:32

Gliese852 mentioned this pull request Dec 10, 2020

The moment of emergence of the hyperspace arrival cloud is not defined. #5067

Closed

Gliese852 force-pushed the tradeships branch 3 times, most recently from 90f15ea to 6bab35d Compare December 13, 2020 08:05

Gliese852 force-pushed the tradeships branch from 6bab35d to b8b9425 Compare January 3, 2021 19:08

Gliese852 force-pushed the tradeships branch from b8b9425 to fb60c94 Compare January 19, 2021 04:06

Gliese852 force-pushed the tradeships branch from fb60c94 to 31d8931 Compare February 6, 2021 20:01

Gliese852 force-pushed the tradeships branch 2 times, most recently from dd73b6c to 0539fd2 Compare March 7, 2021 22:49

Gliese852 marked this pull request as ready for review March 7, 2021 23:13

Gliese852 changed the title ~~WIP: Generate tradeships at different points of their routes~~ Generate tradeships at different points of their routes Mar 7, 2021

Gliese852 changed the title ~~Generate tradeships at different points of their routes~~ Improve tradeships Mar 7, 2021

sturnclaw requested changes Mar 8, 2021

View reviewed changes

src/lua/LuaSpace.cpp Outdated Show resolved Hide resolved

src/ship/PrecalcPath.cpp Outdated Show resolved Hide resolved

Gliese852 force-pushed the tradeships branch from 0539fd2 to 8df15ce Compare March 11, 2021 22:53

Gliese852 requested a review from sturnclaw March 11, 2021 23:13

sturnclaw requested changes Mar 20, 2021

View reviewed changes

sturnclaw requested changes Mar 23, 2021

View reviewed changes

Gliese852 force-pushed the tradeships branch 3 times, most recently from d40efe8 to f32efbe Compare April 9, 2021 21:54

sturnclaw requested changes Apr 9, 2021

View reviewed changes

Gliese852 force-pushed the tradeships branch 2 times, most recently from 88d7c05 to c87c21e Compare April 10, 2021 21:50

sturnclaw requested changes Apr 24, 2021

View reviewed changes

Gliese852 force-pushed the tradeships branch from f6e1e7c to 88b1f6e Compare April 26, 2021 19:51

Gliese852 requested a review from sturnclaw April 26, 2021 19:57

sturnclaw approved these changes Apr 26, 2021

View reviewed changes

Gliese852 added 4 commits April 27, 2021 08:57

Fix some bugs appeared

3668901

- Move the drawing of the system map pigui to a separate handler to ensure that it will always be drawn strictly after the SystemView object - Fix the setting of the m_colliding parameter of the ship during deserialization

Gliese852 force-pushed the tradeships branch from 88b1f6e to 3668901 Compare April 27, 2021 06:07

Gliese852 mentioned this pull request Apr 30, 2021

Improve performance with many ships in one system #5166

Merged

sturnclaw merged commit 9795b9d into pioneerspacesim:master May 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve tradeships #4984

Improve tradeships #4984

Gliese852 commented Oct 26, 2020 •

edited

Loading

The-EG commented Oct 26, 2020

Gliese852 commented Oct 26, 2020

impaktor commented Nov 4, 2020

Gliese852 commented Nov 4, 2020

impaktor commented Nov 22, 2020

Gliese852 commented Nov 23, 2020

The-EG commented Nov 23, 2020

Gliese852 commented Jan 4, 2021

Gliese852 commented Feb 6, 2021

sturnclaw commented Mar 8, 2021

sturnclaw left a comment

Gliese852 commented Mar 11, 2021

sturnclaw left a comment

sturnclaw left a comment

sturnclaw left a comment

Gliese852 commented Apr 10, 2021

sturnclaw left a comment

Gliese852 commented Apr 27, 2021

Gliese852 commented Apr 27, 2021

fluffyfreak commented Apr 27, 2021

sturnclaw commented May 2, 2021

impaktor commented May 2, 2021

sturnclaw commented May 2, 2021

Gliese852 commented May 2, 2021

sturnclaw commented May 2, 2021

Improve tradeships #4984

Improve tradeships #4984

Conversation

Gliese852 commented Oct 26, 2020 • edited Loading

TODO

Commits

Add class for pre-calculating spaceship path

Add function PutShipOnRoute

Improve Tradeships.lua

Fix some bugs appeared

Reduce the flow of tradeships

The-EG commented Oct 26, 2020

Gliese852 commented Oct 26, 2020

impaktor commented Nov 4, 2020

Gliese852 commented Nov 4, 2020

impaktor commented Nov 22, 2020

Gliese852 commented Nov 23, 2020

The-EG commented Nov 23, 2020

Gliese852 commented Jan 4, 2021

Gliese852 commented Feb 6, 2021

sturnclaw commented Mar 8, 2021

sturnclaw left a comment

Choose a reason for hiding this comment

Gliese852 commented Mar 11, 2021

sturnclaw left a comment

Choose a reason for hiding this comment

sturnclaw left a comment

Choose a reason for hiding this comment

sturnclaw left a comment

Choose a reason for hiding this comment

Gliese852 commented Apr 10, 2021

sturnclaw left a comment

Choose a reason for hiding this comment

Gliese852 commented Apr 27, 2021

Gliese852 commented Apr 27, 2021

fluffyfreak commented Apr 27, 2021

sturnclaw commented May 2, 2021

impaktor commented May 2, 2021

sturnclaw commented May 2, 2021

Gliese852 commented May 2, 2021

sturnclaw commented May 2, 2021

Gliese852 commented Oct 26, 2020 •

edited

Loading