Refactor docking #1039
Refactor docking #1039
Conversation
royfalk
requested review from
evertvorster,
BenjamenMeyer and
stephengtuggy
and removed request for
evertvorster and
BenjamenMeyer
|
Tested this with a fresh install, starting a new campaign. Launched my Llama and flew from Atlantis to Serenity and the new docking functionality worked perfectly. The distance to dock started displaying well out from the station and stayed displayed until the Dock:Ready message appeared at 5000m as noted above. As soon as it said Dock:Ready, I was able to dock without issue. However, the return flight to Atlantis did not go as expected. The distance to dock started displaying well out from the plante, and the Dock:Ready message appeared at 5000m, but I was not able to actually dock until 1000 m out (which is what the distance has always been for Atlantis). Also of note, the distances between planets are different now with this PR merged. Atlantis to Serenity are now 5.90 light seconds apart, where it used to be something like 98 light minutes. The distance counts down slowly like it used to when it was light minutes, even though it says light seconds. When travelling at 97.4C, the light seconds used to count down very quickly. Also, when it switches from light seconds to kilometers, the distance in kilometers is much larger than it ever was. I'm not sure any of this is an problem or not - maybe the old distance numbers were wrong? I thought it best to report it all the same, since it is different than what it used to be. Finally, not new to this PR, but I've never heard a docking sound play. I didn't realize there even was one. This is probably related to issue #1015, which I reported previously. |
|
every last piece of equipment seems to have |
|
Atlantis range to dock
It starts at 100km from docking point for both planets and other.
This is a bug I introduced. I divided by 1000 to get km. 5.9 * 1000 = 98 light minutes.
You are probably on the 0.9 assets. If not, please report the stdout/console/log of the error. It'll tell me if it's missing assets or a bug of some sort. |
I'll try and fix this and #1038 today. |
i note the actual dockable radius of atlantis to be 7500 KM so that holds
i checked with a freshly downloaded and unzipped assets-production-master, i stopped getting this error. |
|
on a game play feel note. i love the new docking radii, even the station docking radius of 5KM. though i also kind of liked the station docking minigame. |
|
The distance is now correct and performs as expected when it's showing light minutes and light seconds. However, when I'm close enough to a target to have it show in kilometers, the number displayed is incorrect. For example, when I launch from Atlantis, the distance to Atlantis (right after launching) is 9,222,001Km (but if I turn my ship around for a visual, the planet is right there, close as ever) and the distance to Ataraxia is 700,067,133Km. It's almost like the decimal place is off by 1,000 when the distance is shown in kilometers. All the other planets and bases, which are light minutes or light seconds away show the correct distance. When I flew back to from Serenity to Atlantis to dock, the distance showed as approx. 7,500,000 km when I was able to dock, which further leads me to believe the decimal place is off by 1,000. I'm now having trouble with autopilot (unable to smoothly approach planet - keep activating and deactivating and overshooting the planet) and the effect of gravity wells seems to have disappeared also, but I'm thinking these are both caused by the incorrect distance in kilometers? |
Thank you for the clarification. So docking at Atlantis used to be at 1,000 km, but it's now at 7,500 km? I'm definitely not objecting to the change. In fact, I rather like it. One thing to note (maybe for a future patch), but given the extended docking distances, the docking squares that show when you request docking are now irrelevant, since ships can dock well before reaching those squares. Again, in case I wasn't clear, I really like the way docking works now. Thank you! |
i kind of liked the ones on the station. getting in those squares was a fun little minigame, some of the stations even have their own little challenge mode with internal docking spaces for smaller ships. lent it some realism too. |
|
there is definitly something up with serenity though. it's shields never recharge. may or may not have something to do with the fact my shields take damage on every trip i take. plainfield doesnt have shields and ataraxia almost never takes a hit so i cant speak to any other station. |
|
sorry my last couple comments are out of date. i wasnt aware the code had been updated. this updatye throws the atlantis docking ready distance waaay the hell out there |
This part should be reported under issue #1038 rather than here, as it's a separate issue. |
i have moved it. |
| Transform(dock->GetTransformation(), | ||
| dock->pImage->dockingports[j].GetPosition().Cast())), | ||
| dock->pImage->dockingports[j].GetRadius())) { | ||
| // We cannot dock if we are already docked |
There was a problem hiding this comment.
Shouldn't "already docked" be checked right at function start ?
There was a problem hiding this comment.
Again, this is legacy code and comment.
There was a problem hiding this comment.
OK, then please disregard my question
|
|
||
| bool CrashForceDock(Unit *thus, Unit *dockingUn, bool force) { | ||
| /*bool CrashForceDock(Unit *thus, Unit *dockingUn, bool force) { |
There was a problem hiding this comment.
Welcome to Vegastrike. There should probably be a document somewhere (@BenjamenMeyer ), or maybe not.
I don't know how much you know about the game.
It was originally developed by Daniel Horn and a couple of other people in 1999-2005 or so. Somewhere along the way, I think efforts shifted toward making the Privateer assets work on the engine.
And then people moved on and development sort of stopped.
In 2019 a couple of people came together and restarted development. But the technical debt due to time, loss of knowledge and plenty of other reasons was significant.
CrashForceDock is something from the original devs. Like the comment "We cannot dock if we are already docked". I try not to remove these unless I know for sure it shouldn't stay. As this PR is a quick bug fix, I'm keeping changes to a minimum.
However, as I noticed this function is unused, I commented it out.
There was a problem hiding this comment.
Eh, either you're keeping changes to a minimum or you do additionnal things, like commenting out dead code... ;-)
I already worked a bit on VS in the (distant) past, and keep comig back to it, but I know almost nothing about the code. All my comments are from this naive perspective. Don't take them as harsh or anything. They're mostly just questions.
I'm not saying that you're wrong, but reviews are easier if you keep the changes to the bare minimum. This IMHO should be done in another cleanup PR, or at least a separated commit that does just this "remove dead code". There's no need to keep it, it is in the git history anyways. As a separate PR this trivial thing can be merged easily and independently of the bug fix.
There was a problem hiding this comment.
Eh, either you're keeping changes to a minimum or you do additionnal things, like commenting out dead code... ;-)
I know. The temptation.
I already worked a bit on VS in the (distant) past, and keep comig back to it, but I know almost nothing about the code.
@BenjamenMeyer can give you dev privileges, if you'd like.
I'm not saying that you're wrong, but reviews are easier if you keep the changes to the bare minimum. This IMHO should be done in another cleanup PR, or at least a separated commit that does just this "remove dead code".
I agree. Unfortunately, due to the excessive technical debt, we have some truly monstrous PRs. And there is a cost to doing things cleanly. I'm already juggling several PRs, in various stages of readiness.
There's no need to keep it, it is in the git history anyways.
This is true for people with functioning memories. I introduced a bug by merging an engine PR and forgetting there's a complementing assets one.
There was a problem hiding this comment.
I have to agree with @vincele here. As a general rule, do not make functional changes in the same PR with refactor changes. It makes for huge, difficult-to-follow PRs, and a nightmare in case we need to go back and revert any of the functionality changes. You've done way too much of this in the past, and I am not happy about it. I should have reviewed your past PRs more carefully.
And if you're going to take out sections of code, don't bother commenting them out. Just remove them completely -- at an appropriate time, i.e. in their own PR.
| } | ||
|
|
||
| if (CanDock(station, target, true) != -1) { | ||
| if (CanDock(station, target, false) != -1) { |
There was a problem hiding this comment.
This reads a bit strange, maybe a comment could help a casual reader
There was a problem hiding this comment.
Huh. The original code was:
if (station->CanDockWithMe(target, true) != -1) {
if (station->CanDockWithMe(target, false) != -1) {
I've no idea why it this so.
You are most welcome to provide comments. I generally strive to get a few yays, especially from the testers as they find all the nasty surprises left in the code. |
|
Did further testing tonight on this PR, with the following results: Running a version of master with PR#1036, 1039, 1047 and 1048 cherry-picked and merged. Started a new campaign and launched from Atlantis. Distance from Atlantis showed as 9221980 kms (which doesn't look right). Distances to all other nav points looked correct (some in light minutes and some in light seconds) except for Ataraxia which showed as 70067125 kms. Flew from there to Serenity. The distance to docking countdown started at 100000 kms (which seems high) and the docking ready indicator showed up at 5000 (which is perfect). Interestingly enough, as it was a patrol mission, I had to get to 200 meters from Phillies before the scanning would complete (which is a lot closer than it used to be). Flew from Serenity to Phillies. Distance to target dropped as expected from light minutes thru light seconds but when it dropped to kilometers the distance was 299792458 kms. I got to about 260000 kms and the distance to docking countdown came on showing 100000 meters, but Phillies blew up in front of me so I couldn't finish that. However, I assume the docking ready would have shown at 5000 meters (as it did for approaching Serenity) Flew from there to Plainfield (a relay station). Again, dropped out of light seconds 299792458 kms away (which is too high). I got to 100000kms way when the distance to docking count down started (seems too high). Docking ready indicator came on at 5000 meters (which is perfect). Unfortunately, I was approaching closer in order to scan the station (for my patrol mission) when the game froze for a second or two and then crashed to the desktop. I'll try again tomorrow. I especially want to test approaching Atlantis as I wasn't sure about the distances the last time I played and I want to make sure I have an accurate record. |
definitly a multiplication issue on 299792458. that is the exact speed of c in game. presumale in meters per second. though maybe not since. however it also appears here.
line 578 in Vega-Strike-Engine-Source/engine/src/gfx/vdu.cpp of at least the master branch of the engine source. |
- light_second now takes value from physics.h - Distance of two objects only takes radius of planets into accounts. Return to old calculation. - CanDock refuses to dock with ships without docks or jump points. - GetDockingText ignores ships without docks or jump points. - Hard coded values in GetDockingText now refer to configuration. - Remove abort from function. - Fix issue in PrettyDistanceString - km did not divide distance by 1000 - Add missing new line at end of files - Fix indentation in configuration - Add missing space in vdu
yeah same, you're never going to convince me that digging your way out of a planetary gravity well is part of the fun. it's simply a grind to me the sort of thing added to a game so a micro transaction could make it disappear. |
|
Interesting. I've been looking through the old open issues to see if any can be closed, and noticed that #37 has an extensive discussion about docking and possible changes. That issue was opened in 2020, so docking certainly isn't a new issue! |
|
@kheckwrecker You are correct. This subject has been under discussion for quite a while. OK, how about if we change the docking behavior for planets only, and leave the station docking as-is for now? I would be OK with that. How about you guys? |
Just to be clear what we're agreeing on::
If so, then I'm good with your suggestion. Incidentally, there are two more old open issues on docking: #212 - Cannot Dock and #267 - Docking indicator cuts in and out during approach Probably should link those to PR so they close when the PR (in its final form) is merged with master? |
this sounds like awesome to me. oh hey so my initial problem of "even just fixing the docking indicator." was already a known and open issue. |
|
Gravity wells are supposed to suck. Quite literally! It adds realism to the game. It is what sets Vega Strike apart from Oolite and similar games. As you learn a couple of tricks, even in the slower ships the gravity wells impact can be greatly mitigated, which is yet another reason to leave them as they are. Now what really does suck without any hope of getting it better in the game is the fact that planetary docks can be occupied. It would be nice if one could purchase a priority pass in the game that allows you to dock when in range, and kicks out whoever is docked at the moment. :) In my opinion, the docking distance should be a configurable option. |
|
I'm not sure if we know how to fix #212 and #267 yet. Unfortunately.
I could go for that. Seems like everybody wins that way. We can probably introduce a setting specifically for the planet docking distance, and have it default to 7500 km like this PR already has it. Then, for now at least, we still require flying through the little green squares to dock with stations. To be revisited later, potentially.
If we know how to, sure.
Yep.
Yes, exactly. |
try afterburning your way out of the atlantis gravity well in a mule, or into atlantis for a trade run for that matter. the mule is a bigger more expensive ship and if you fly one you will never want to land on a planet ever again. it is worth noting that wing commander privateer a spiritual predecessor to this game skipped docking sequences entirely and also skipped gravity well issues all together. and that wasnt a forced issue it was a design choice. people saying that getting rid of the gravity well issue for docking will kill the game should maybe look back at some of the predecessors and their design choices. because some of them thought otherwise and they werent wrong. |
|
It feels like I have been over this a few times, but maybe I have not
explained my position clearly enough.
try afterburning your way out of the atlantis gravity well in a mule, or
into atlantis for a trade run for that matter. the mule is a bigger more
expensive ship and if you fly one you will never want to land on a planet
ever again.
Which is sort of the point. If you drive a tanker, you can expect it to be
slow.
Really, the realism is what sets Vega Strike apart from the other "space
simulators".
If the realism goes away, then so does the want to play Vega Strike.
I would just play No Man's Sky or Elite Dangerous.. or Oolite. All of those
have better graphics, and are way more polished, but none has the realistic
feel of Vega Strike.
Frankly, I have never understood the need to make all the games of a
similar genre to play the same way. Variety is the spice of life, after all.
To get back to your mule example, did you know that you can pay someone to
do the tedious flying for you? You can just zip ahead in a small fast craft
to find deals, then call the mule and trade. It costs a bit of money, but
gets rid of the tedium of driving your tanker out of a deep gravity well.
I have a youtube video on how to do this, if you are interested.
In the beginning of the game, when you don't have a lot of cash, you have
to deal with the inconveniencies of having a horribly overloaded Llama and
deep gravity wells. It makes you re-evaluate your choices of cargo, etc..
it all adds to the realism of the game. Making this not suck makes the game
boring.
The quickest way out of a gravity well is to put it right at your back, hit
"Y" to put your computer into travel mode, adjust thrust to fulle speed
(You'll see it is three orders of magnitude faster than maneuver mode) hit
Shift+A to switch on SPEC. And then wait a little. This is a good
opportunity to look around, make sure that your destination is locked in
missions are prepped that sort of thing. You can afterburn here, but it
does not make much of a difference.
Once the SPEC multiplier starts ticking it's really not a long time before
you are out of the well and can switch to automatic navigation. Don't
forget to switch your computer back to maneuver mode or you'll be likely to
make a smear on the side of your destination...
it is worth noting that wing commander privateer a spiritual predecessor
to this game skipped docking sequences entirely and also skipped gravity
well issues all together. and that wasnt a forced issue it was a design
choice.
It is also worth noting that Vega Strike is a _different game_, with
different goals.
One of the main goals is realism. Think of it more like "MS Flight
Simulator for space", rather than Wind Commander, which are arguably some
of the most unrealistic space games out there.
people saying that getting rid of the gravity well issue for docking will
kill the game should maybe look back at some of the predecessors and their
design choices. because some of them thought otherwise and they werent
wrong.
I'm saying, let's make this a configurable option, and we both can have
what we want.
I'm even willing to bet you a whole 50 cents that in time you will see that
I am right on this issue.
Peace.
|
Point by Point
I'll take care of that. You really don't need to merge this or other PRs on top of master to test. Just test the branch itself.
I'm pretty sure this fixes #267. I wasn't aware of 212 and did not touch the key mapping. However, I believe @evertvorster mentioned a how-to in the issue.
All good. We've all had these.
It is.
I take no sides in this discussion of realism/fun, except where it impacts the code itself. Where possible, I make this configurable in the settings.
Please separate the engine from the game. VS is realistic. The engine is supposed to support fun games like privateer as well. |
Proposal
What do you guys think? Issues:
|
|
I think you are doing a stellar job, Roy. Thank you for all the effort you put in. I agree with your comment about Vega Strike the game versus Vega Strike the game engine, and this of course necessitates making quite a lot of things configurable. At some point in the future, it could be a feature in Vega Strike the game where you could buy some sort of docking automation, or pay an expedited docking fee that gets you to do the simple dock. For now, the mechanism to achieve this exists, and that is really good. |
That was the original problem. This branch will not compile due to the conflicts, so I have no way to test the branch itself. Merging PR's onto master was my attempt to get around this problem. Unfortunately, that didn't work either. I'll keep an eye out for your fixes to the conflicts. |
This all looks good to me. Thanks for all your hard work on this PR and on this game in general. |
try afterburning your way out of the atlantis gravity well in a mule, or into atlantis for a trade run for that matter. the mule is a bigger more expensive ship and if you fly one you will never want to land on a planet ever again. it is worth noting that wing commander privateer a spiritual predecessor to this game skipped docking sequences entirely and also skipped gravity well issues all together. and that wasnt a forced issue it was a design choice. people saying that getting rid of the gravity well issue for docking will kill the game should maybe look back at some of the predecessors and their design choices. because some of them thought otherwise and they werent wrong.
yes i did know all of this, and the fastest way to get out of a gravity well isnt just to enter maneuver mode but to turn on slide mode and just hit the afterburner. but the problem comes with the fact that none of that is fun and it still takes forever. as for the configurable, sure i am good for that. as for the 50 cents? nah i am having more fun on this docking refactor branch then i have had ever with vegastrike. the thing presently killing my fun isnt even related to this it's the equipment selling bugs that are currently being experienced. |
This is no longer true. It was ridicules that planets had one parking spot and I replaced that with the unlimited dock size.
It is for planets (always) and stations (simple mode).
Can you please link to an issue and open one if need be with details.
This was never a feature of this PR. Whoever asked for it, please open another issue. |
#1038 |
|
like i said i hadnt had a chance to try the new commit yet. |
|
i got some time today so lets try this latest commit. |
Well, that was unexpected. I'll try and test this over the weekend and see if I can figure out what changed. I've been making incrementally smaller changes. The only big one was the return to classic docking for stations. |
|
yeah i flew out to over 5 light hours and i could still dock with a station. i dont know what the actual range is but i suspect any station would die before i found ot. |
I'm having this same problem. It's fun to dock with a station after launching from Atlantis. Quick way to make a lot of money when you don't actually have to fly anywhere! |
|
The other problem I'm having with this PR is that docking ranges displayed for planets are not the actual docking distance. For example, for Atlantis, the docking countdown starts at 7500 km (good) and it says Docking: Ready at 2500 km, which is good, but you can't actually dock at that distance. In fact, you can't actually dock until you reach 1000 km, while it says Docking: Ready all the way in. Interestingly enough, 1,000 km is the original docking distance for Atlantis, so that hasn't actually changed with this PR. Tested this by also flying to Phillies. I can't tell you what distance Docking: Ready first appeared, but I couldn't actually dock until 339 km out from the planet, which is again the original docking distance for Phillies. I can test the other planets in Cephid-17, but I'm pretty confident they'll all work the same way. |
i was not experiencing this issue. i was easily docking with atlantis around the 2400 to 2500 km range. |
stephengtuggy
dismissed
their stale review
Dismissing my review, as we have since come to an agreement on how this code should behave.
Code Changes:
Issues: