Elite radar

Talk among developers, and propose and discuss general development planning/tackling/etc... feature in this forum.
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

I tried to upload the diff to the bug tracker, but it was rejected because the gzipped archive is 280Kb and the bug tracker only allows 256Kb.

It appears that the archive is this big because (1) svn did a really bad job at making a diff of the master_part_list.csv and units.csv files, and (2) it contains an image.

Please advice.
charlieg
Elite Mercenary
Elite Mercenary
Posts: 1329
Joined: Thu Mar 27, 2003 11:51 pm
Location: Manchester, UK
Contact:

Re: Elite radar

Post by charlieg »

Attach here. Or attach the image separately.
Free Gamer - free software games compendium and commentary!
FreeGameDev forum - open source game development community
ace123
Lead Network Developer
Lead Network Developer
Posts: 2560
Joined: Sun Jan 12, 2003 9:13 am
Location: Palo Alto CA
Contact:

Re: Elite radar

Post by ace123 »

I'm getting a bit worried about your patch for a couple reasons:
Firstly, it sounds like you did a lot of cleaning up in other parts of the code. This is great, and we welcome such changes, but having them mixed in with useful features is really dangerous, because it's impossible to dig through a 2000-line diff for the 200 important lines which actually change the radar. That being said, there are legitimate reasons to combine them, and I understand if it's hard to separate the changes, especially if your radar depends on the code cleanup. This is largely up to your judgement.

Secondly and most importantly, your code change should not depend on a change to *any* data files. The changes to data should only serve to make your code work better.

For example, I am not happy if Elite radar *breaks* standard radar for all ships without the change to master_parts_list.csv; but I'm happy if you change it so standard radar *always works*, even if your new Elite radar will only turn on for ships which have it. In other words, we need the game to be still playable for people using a model/dataset with no changes.

The reason I want to be strict about this is because Vega Strike has several mods which use the same engine, and if we make a backwards-incompatible change (changing code *and* data in the same commit), then other mods, such as Parallel Universe may suddenly lose their radars.

Therefore, it should be possible to commit purely a code change (only svn diff vegastrike/src), and once that is reviewed and committed, we can take another pass at the data changes, to enable the cool new radar for some ships, as a *separate* commit.

This will also solve the problem of the patch size, since you are starting by only changing code, and the data files can just be zipped up at a later point.
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

charlieg wrote:Attach here. Or attach the image separately.
Here we go...
You do not have the required permissions to view the files attached to this post.
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

ace123 wrote:I'm getting a bit worried about your patch for a couple reasons:
That is a lot of concern about something you have not seen yet.
ace123 wrote: Firstly, it sounds like you did a lot of cleaning up in other parts of the code. This is great, and we welcome such changes, but having them mixed in with useful features is really dangerous, because it's impossible to dig through a 2000-line diff for the 200 important lines which actually change the radar. That being said, there are legitimate reasons to combine them, and I understand if it's hard to separate the changes, especially if your radar depends on the code cleanup. This is largely up to your judgement.
All changes in the patch are needed by the new radars. There is a one-line change (src/cmd/unit_collide.h) that was not needed by the radar refactoring, but since it was a programming error I thought it prudent to include it. I am willing to negotiate this line though :wink:.

Only GameCockpit has been heavily refactored in this patch, but that was unavoidable.

All other changes to the existing classes have been minimal. They have mainly been introductions of helper functions (e.g. Unit::IsBase()), renaming of variables (e.g. RADARLIM::iff has become RADARLIM::capability because it contains other capabilities than IFF), and modifications to read the configuration files correctly.
ace123 wrote: Secondly and most importantly, your code change should not depend on a change to *any* data files. The changes to data should only serve to make your code work better.
My changes are, to the best of my knowledge, backwardscompatible. My code patches should work with the old data files. However, my data patches are necessary to activate the new radars, so the code changes are worth little without the data changes.

Furthermore, have a look at the readme.txt file in the patch, where I have described both the technical changes and the changes that the user will experience.

While we are on the topic of backwardscompatibility, I just discovered that there is a bug in my patch in the network code. This patch fixes it:

Code: Select all

diff --git a/src/networking/zonemgr.cpp b/src/networking/zonemgr.cpp
index 94b37d2..92d96f0 100644
--- a/src/networking/zonemgr.cpp
+++ b/src/networking/zonemgr.cpp
@@ -796,7 +796,7 @@ void ZoneMgr::addDamage( NetBuffer &netbuf, Unit *un )
     }
     if (damages&Unit::COMPUTER_DAMAGED) {
         netbuf.addChar( un->computer.itts );
-        netbuf.addInt32( un->computer.radar.capability );
+        netbuf.addChar( un->computer.radar.UseFriendFoe() ? 1 : 0 );
         netbuf.addFloat( un->limits.retro );
         netbuf.addFloat( un->computer.radar.maxcone );
         netbuf.addFloat( un->computer.radar.lockcone );
klauss
Elite
Elite
Posts: 7243
Joined: Mon Apr 18, 2005 2:40 pm
Location: LS87, Buenos Aires, República Argentina

Re: Elite radar

Post by klauss »

Sorry for the long hiatus here, I've been having trouble with my computer at home.

I'll take a look at the patch as soon as I can, downloading it now.

Yep, I didn't mean to sound discouraging. As our previous conversations, you know I mostly like your patch. Ace's concerns are valid, though, it would be best to avoid data-side changes, or at the very least, produce intermediate commits that don't require them, to ease further maintenance tasks and possibly debugging - it's all too common to commit something that works in my distro but not on someone else's (or in windows, or mac).

I know this patch is rather basic and unlikely to break in a platform-dependant fashion, but you probably get my meaning.

I'll try to break the patch into functional pieces and, assuming there's no big issue, commit them. It was almost there last time I reviewed it, and if you did address my main concerns it's probably good to go already.
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Patch manually added in
Compiling now...
Errors...

Can you post an svn patch for this?
Its late and im off for the night.

I will post the errors tomorrow and see if I can't track em down. Get me that svn diff quick as you can please.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

pheonixstorm wrote:Patch manually added in
The venerable patch command can do this automatically for you.

It is available for Windows at http://gnuwin32.sourceforge.net/packages/patch.htm. It will also handle my first diff. Notice that you may have to add the "--binary" option on Windows.
pheonixstorm wrote:Can you post an svn patch for this?
Attached.

Remember that this is a diff against the SVN trunk.
You do not have the required permissions to view the files attached to this post.
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Ok, so manual or patch I get the following errors on a win build (not including sys/time.h)

Code: Select all

Compiling...
sphere_display.cpp
plane_display.cpp
bubble_display.cpp
..\..\vegastrike\src\gfx\radar\bubble_display.cpp(185) : error C2780: 'const _Ty &std::max(const _Ty &,const _Ty &,_Pr)' : expects 3 arguments - 2 provided
        e:\Programming\VS9\VC\include\xutility(3390) : see declaration of 'std::max'
..\..\vegastrike\src\gfx\radar\bubble_display.cpp(185) : error C2782: 'const _Ty &std::max(const _Ty &,const _Ty &)' : template parameter '_Ty' is ambiguous
        e:\Programming\VS9\VC\include\xutility(3382) : see declaration of 'std::max'
        could be 'float'
        or       'double'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2143: syntax error : missing ')' before ','
..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2660: 'Radar::DualDisplayBase::SetViewArea' : function does not take 0 arguments
..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2059: syntax error : ')'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(152) : error C2143: syntax error : missing ';' before '->'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(202) : error C2513: 'const float' : no variable declared before '='
..\..\vegastrike\src\gfx\radar\plane_display.cpp(203) : error C2513: 'const float' : no variable declared before '='
..\..\vegastrike\src\gfx\radar\plane_display.cpp(206) : error C2059: syntax error : '/'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(207) : error C2059: syntax error : '/'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(208) : error C2059: syntax error : ')'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(365) : error C2780: 'const _Ty &std::max(const _Ty &,const _Ty &,_Pr)' : expects 3 arguments - 2 provided
        e:\Programming\VS9\VC\include\xutility(3390) : see declaration of 'std::max'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(365) : error C2782: 'const _Ty &std::max(const _Ty &,const _Ty &)' : template parameter '_Ty' is ambiguous
        e:\Programming\VS9\VC\include\xutility(3382) : see declaration of 'std::max'
        could be 'float'
        or       'double'
For the first error I can be rid of it by changing
float trackSize = std::max(1.0, log10(track.GetSize()));
to
float trackSize = std::max(1.0, log10<float>(track.GetSize()));

The rest im not so sure on.. trying to track them down now.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Took a little while to find all the problems but I did.

First:

Code: Select all

\vegastrike\src\gfx\radar\bubble_display.cpp(185) : error C2780: 'const _Ty &std::max(const _Ty &,const _Ty &,_Pr)' : expects 3 arguments - 2 provided
        e:\Programming\VS9\VC\include\xutility(3390) : see declaration of 'std::max'
..\..\vegastrike\src\gfx\radar\bubble_display.cpp(185) : error C2782: 'const _Ty &std::max(const _Ty &,const _Ty &)' : template parameter '_Ty' is ambiguous
        e:\Programming\VS9\VC\include\xutility(3382) : see declaration of 'std::max'
        could be 'float'
        or       'double'
This section along with the second set from line 365 I solved by changing

Code: Select all

float trackSize = std::max(1.0, log10(track.GetSize()));
to
float trackSize = std::max<float>(1.0, log10(track.GetSize()));
The next section:

Code: Select all

..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2143: syntax error : missing ')' before ','
..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2660: 'Radar::DualDisplayBase::SetViewArea' : function does not take 0 arguments
..\..\vegastrike\src\gfx\radar\plane_display.cpp(149) : error C2059: syntax error : ')'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(152) : error C2143: syntax error : missing ';' before '->'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(202) : error C2513: 'const float' : no variable declared before '='
..\..\vegastrike\src\gfx\radar\plane_display.cpp(203) : error C2513: 'const float' : no variable declared before '='
..\..\vegastrike\src\gfx\radar\plane_display.cpp(206) : error C2059: syntax error : '/'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(207) : error C2059: syntax error : '/'
..\..\vegastrike\src\gfx\radar\plane_display.cpp(208) : error C2059: syntax error : ')'
was a little more difficult to figure out. I tried and tried to find missing items (as stated) but it turned out that anywhere in the code you named something near or far VC was looking in WinDef.h at #define near and #define far. To correct this I renamed anything near and far to Rnear and Rfar.

For the sys\time.h I added the following:

Code: Select all

#ifndef WIN32
#include <sys\time.h>
#else
#include <time.h>
#endif
Now, odd thing with time.h In the VC includes there is a include\sys\utime.h and a include\sys\timeb.h but the only time.h is in includes\

Well, it compiles now so about to test it. If it works, work the fixes in however you want and I will up the adjustments or I can leave as is with my changes.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

First of all, good work!
pheonixstorm wrote:This section along with the second set from line 365 I solved by changing

Code: Select all

float trackSize = std::max(1.0, log10(track.GetSize()));
to
float trackSize = std::max<float>(1.0, log10(track.GetSize()));
I suggest that we use this variant (notice the f suffix on the number):

Code: Select all

float trackSize = std::max(1.0f, std::log10(track.GetSize()));
pheonixstorm wrote:I tried and tried to find missing items (as stated) but it turned out that anywhere in the code you named something near or far VC was looking in WinDef.h at #define near and #define far. To correct this I renamed anything near and far to Rnear and Rfar.
Very nasty error.

Minor suggestion, could we please use nearSprite, nearDistance and farDistance as the new names? I prefer self-documenting variable names wherever possible, and a prefixed R is not very intuitive.
pheonixstorm wrote: For the sys\time.h I added the following:

Code: Select all

#ifndef WIN32
#include <sys\time.h>
#else
#include <time.h>
#endif
Just nitpicking, but on Linux the path separator is / not \.

Actually, I do not believe that the time header is needed anymore. I suggest that you simply remove it.
pheonixstorm wrote:Well, it compiles now so about to test it. If it works, work the fixes in however you want and I will up the adjustments or I can leave as is with my changes.
The radars can only be bought in a few places, and only if you remember to update the data files.

If you want a quick and dirty way to test the radars, I suggest that you temporarily modify the switch statement in GameCockpit::OnDockEnd() to use the radar you want to test. BubbleDisplay is the Rlaan radar, and PlaneDisplay is the Highborn/Elite radar.
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

I just wanted a quick fix to get it working so adding R to near and far was quickest (R for Radar lol). I will edit the pages again and commit to my branch.

Testiing in game I just bought the the 2610 you added in. Not sure if I like the crosshairs yet, guess it'll have to grow on me. Didn't you say that there were vegastrike.config changes to get some of the code turned on? Good work though.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Ok, values changed and recompiled. I left the time.h alone for now.

About to svn up the bunch in an hour or two. One note though, Klauss is right about the smaller patches. Do try to make them smaller ;) Or at least break them down into smaller code changes. Will make it easier to track down bugs when compiling on windows.

Also, for whatever reason tortoisesvn did not like dev/null on the patch and refused to patch the new files. I replace the new file mode and dev/null with --- /path/newfile.etc Annoying but not a big issue for me.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

pheonixstorm wrote:Testiing in game I just bought the the 2610 you added in. Not sure if I like the crosshairs yet, guess it'll have to grow on me.
At the time it seemed like a good solution, but I am open to changes.

I modelled the new Confed radar crosshairs after the radar target marker (the symbol that marks which target you are locked on to), which is a cross, in order to make them more consistent. This change should be seen in a larger context. The Rlaan Tracker uses octagons as radar crosshairs and target markers. The Highborn radar does not have radar crosshairs at all, and its target marker is a 3D line from the center to the target.

I am currently experimenting with alternative targetting and docking boxes for the different radars. The Confed radars will use the targetting/docking boxes as we already know them. The Rlaan Tracker uses an octagon instead of a square for the targetting boxes, which happens to be the same as its radar crosshairs. Following this line of reasoning, we could change the Confed radar crosshairs to have a squared shape instead. However, if we want to have consistent symbols then we should also change the radar target indicator from a cross to a square.
pheonixstorm wrote:Didn't you say that there were vegastrike.config changes to get some of the code turned on?
Nope, quite to the contrary. The hud/radarType setting is no longer used. Such testing settings tend to obscure the code.

Instead I have saved games with the various radars (which I bought by temporarily changing the units.csv file to make the upgrades available at the type of station I have docked at.) I acknowledge that this is more of a hassle to set up, but it keeps the code clean.
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Sounds good. The jittering hyper dots are funny to look at. In one new game 6 of them dancing away. Looked like a I had a radar waltz going on.

One thing I did notice, on entering bernards star the space background interferes horribly with the radar making it difficult to see. This was on the llama and robin cockpits. Can you set the radar background to a solid black?
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

While i'm thinking about it. Does the radar show objects that are only in range or everything in the system? I know targeting will pull up anything and everything and wanted to know if the two are seperate or linked. If seperate do you think you can correct the target data to only pull up whats in the radar data.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

pheonixstorm wrote:One thing I did notice, on entering bernards star the space background interferes horribly with the radar making it difficult to see. This was on the llama and robin cockpits. Can you set the radar background to a solid black?
I agree that the current panel is too transparent. We ought to ask the graphics designers though. They are better qualified than me.
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

pheonixstorm wrote:While i'm thinking about it. Does the radar show objects that are only in range or everything in the system?
I think that it shows (uncloaked) objects within radar range, plus planets and other gravitational objects. I have reused the existing code for finding radar blips, so I am not entirely certain, though.
pheonixstorm wrote:I know targeting will pull up anything and everything and wanted to know if the two are seperate or linked. If seperate do you think you can correct the target data to only pull up whats in the radar data.
They are separate, and yes, I think that we can change it to only pick what is in radar range, although that would probably mean refactoring other parts of the codebase.
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

Well, work on what you feel up for. No telling when klauss and chuck will be back around to get to the graphic issue on the radar though.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
klauss
Elite
Elite
Posts: 7243
Joined: Mon Apr 18, 2005 2:40 pm
Location: LS87, Buenos Aires, República Argentina

Re: Elite radar

Post by klauss »

Hey :D

Back. Didn't really fix my computer, but it's patched enough to let me do some stuff.

So, phoenixstorm, were you able to apply the patch?

I was going to before my compy died on me, and now that I'm back up I was gonna to, but if you have a newer patch I might try that one instead. I did notice (by virtue of svn up) that you didn't apply it to trunk at least ;)
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
pheonixstorm
Elite
Elite
Posts: 1567
Joined: Tue Jan 26, 2010 2:03 am

Re: Elite radar

Post by pheonixstorm »

yeah, I applied this and the mesh animation code on my branch. Wanted to test it out before I sent it on the trunk. No bugs i've run across for either patch so far. Both seem to be stable so by the middle of the month I will have my changes merged with trunk more than likely.
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
Shark
Confed Special Operative
Confed Special Operative
Posts: 360
Joined: Tue Mar 02, 2004 9:34 am
Contact:

Re: Elite radar

Post by Shark »

Homeworld had a nice feature where you hit the spacebar and you would zoom out to "Sensors manager" where you would get a wider view of the surrounding area and where objects (except for very very large ones) were reduced to sensors "blips". Might be nice to have that in combination with the existing external views in VS, as it was an awesome feature.
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

Shark wrote:Homeworld had a nice feature where you hit the spacebar and you would zoom out to "Sensors manager" where you would get a wider view of the surrounding area and where objects (except for very very large ones) were reduced to sensors "blips". Might be nice to have that in combination with the existing external views in VS, as it was an awesome feature.
This sounds like a good idea, and one that fits very well with my wish to create a view with a tactical overview.

I was not aware of the Sensors Manager in Homeworld. Thanks for the reference.
klauss
Elite
Elite
Posts: 7243
Joined: Mon Apr 18, 2005 2:40 pm
Location: LS87, Buenos Aires, República Argentina

Re: Elite radar

Post by klauss »

That sounds like the nav computer.

I know... ours is ugly. Feel free to sex it up ;)
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
breese
Bounty Hunter
Bounty Hunter
Posts: 152
Joined: Thu Sep 02, 2010 8:00 pm

Re: Elite radar

Post by breese »

klauss wrote:That sounds like the nav computer.
Or something between the nav computer and the rader. I tried to scale up the Highborn/Elite radar to fullscreen, and it worked rather well.
klauss wrote:I know... ours is ugly. Feel free to sex it up ;)
The System and Sector maps have been on my todo list from the very beginning.

I have a prototype where I have replaced the circular symbols (representing planets, stations, etc.) on the System map with their respective sprites (whose shown in the VDU). It helps to easily identify the different types of planets. However, I have not had much time to experiment further with this.
Post Reply