Map Icons (sugestions needed)

[Mr. Suspicious]

After much struggling with the code, I finally have a "running" version of "changable icons"

Attached you find a tar.gz with the full map.cpp, map.h and icons.conf. Place icons.conf in /usr/local/share/showeq/ dir

As you will notice, it GREATLY reduces spawnmovement on the map. That's due to the spot in the code that it loads the icons from the icons.conf into the class MapIcon and exactly the reason I ask for erm.. "guidance". Currently it reads the file in paintSpawns(). Could anyone look at it and give me a suggestion as to where else would be a good spot (a functionname that's backtrackable to the functioncall would be nice) since I'm about to loose my head figuering this out =P

attachement :rolleyes: :http://www.petervandongen.com/download/map2.tar.gz (32kb)

PS: I've left in 2 lines I use to debug in the code, please be gentle and not mention them =P

[Zaphod]

Here is some quick guidance, feedback, and a few questions:

Learn more about Qt (look at the tutorials and examples in $QTDIR/doc/html/index.html) and use the Qt classes where available.
Learn more about C++.
In general, the use of <iostream.h> and <fstream.h> are deprecated in C++ (use <iostream> and <fstream> instead). Within ShowEQ you should consider using the QFile and QTextStream classes from Qt instead.
The array of icons should be made a member and initialized in the Map class constructor and not in Map::paintSpawns()
You should probably be using QColor's in the MapIcon structure instead of ints that are passed to switch statements
It looks like your iIconLine member of MapIcon is really a true/false value, in which case it should be a bool at the end of the structure.
You should support an option to use the spawn color for the icon pen or brush.
You should use the preferences system instead of a seperate text file to initialize the icon data.
A method of configuring the icons and how they are displayed should be provided from the map menus along with a method of persisting those changes.
Try to stick to the method/member naming style used elsewhere in the code (we don't do polish).
What's the point of the iIconName member of MapIcon?
Why doesn't the TRACER filter have an icon?


I'll take a further look at the code later.

Hope this helps & Enjoy,
Zaphod (dohpaZ)

[Mr. Suspicious]

Well, 4th time I reply on this *sigh* 3 times my ISP went offline just before hitting submit *loooove the wonders of technoloky*

First off, it's only in the "rough structure stage" right now and CERTAINLY NO "patch material", perhaps it would have been 2 years ago, but not the way the code is structured today.

- Learn more about Qt (look at the tutorials and examples in $QTDIR/doc/html/index.html) and use the Qt classes where available.
- Learn more about C++.

That's what I'm doing =) Last time I realy programmed was in Assembler on 8086s (15 years ago), nowadays I only do some php, ASP, VB stuff.

In general, the use of <iostream.h> and <fstream.h> are deprecated in C++ (use <iostream> and <fstream> instead). Within ShowEQ you should consider using the QFile and QTextStream classes from Qt instead.

Ah, didn't know, will do.

The array of icons should be made a member and initialized in the Map class constructor and not in Map::paintSpawns()

Ahhhh, that's where I went wrong. I knew it was in the wrong spot.

You should probably be using QColor's in the MapIcon structure instead of ints that are passed to switch statements

Aye, is in the TODO list, however, ints were the "easy" things to use for the "rough frame stage" it's in now. Also Qcolors will open up the opportunity for a whole range of to use colors.

It looks like your iIconLine member of MapIcon is really a true/false value, in which case it should be a bool at the end of the structure.

Is now, will be a "line" / "noline" / "slashed line" / "dotted line" / "red line with flashy blue slashes" or whatever other lines QT allows (I'm assuming it uses the GD library)

- You should support an option to use the spawn color for the icon pen or brush.

Aye, didn't think of that yet, should be easy enough to implement.

- You should use the preferences system instead of a seperate text file to initialize the icon data.

True, and is planned aswell... in this stage however I wanted to stick with map.cpp for a bit and work in there before leaving the module and messing everything up *g*

- A method of configuring the icons and how they are displayed should be provided from the map menus along with a method of persisting those changes.

Planned aswell, just not a high priority at this moment. Was and still am conserned with the way my code end up on the stack/heap without filling the memory to the notch right now =P Most people wouldn't give it a second thought, but those kinds of things are important for me. Not only making something, but actually understanding how it works in the momory aswell. For the time being I'll reconstruct the Icons already used in the code and add the interface as the very very very last thing.

- Try to stick to the method/member naming style used elsewhere in the code (we don't do polish).

"polish"? Uhm.. like "the language in Poland"? My native language is not English, so I might be missing your point here.

- What's the point of the iIconName member of MapIcon?

Debugging purposes:

cout << "Icon for " << Icons[IconIndex].iIconName << " loaded/n"

Just to see IF it loaded and IF it loaded the correct thing in the correct Icons[section]. Is actually a direct port from a small program I use that emulates the behaviour I want *compiles in seconds compared to the 4 minute recompile of map.cpp and map.h*

- Why doesn't the TRACER filter have an icon?

To quote someone else "WTF is TRACER?", lol. No, in all fairness, all instances will be ported over in the final version (Available here Christmas 2008) to be used in the finished code. Also the PvP and Diety things and whatnot. I just needed to use a "few" as example to see if it works alright.

Also please notice that I only "reconstructed" the original Icons, triangles, diamonds and other possible icons will be added in the end aswell.

Thanks for the reply and taking the time to read through the code Zaphod, I realy appreciate it. If you see some other "daft" things, please let me know. *was secretly hoping someone'd go "HELL NO STOP THE INSANITY NOW! I'LL DO IT!"* anyone? :D

[high_jeeves]

Hehehe.. this one cracked me up...



- Try to stick to the method/member naming style used elsewhere in the code (we don't do polish).

"polish"? Uhm.. like "the language in Poland"? My native language is not English, so I might be missing your point here.


"Polish" (also called Polish Notation, and a few other things) is a way of labeling variables. It is particularly popular (although not exclusively) with Visual C++ coders... It is a quick variable identification method.

For example

int nVariable (the n means integer)
boolean bVariable (the b means boolean)
char* sVariable (the s means string)
float fVariable (the f means float)

object member variables are generally prepended with a m_:

int m_nVariable
float m_fVariable

camel case is generally used (first letter of each word is caps, all other letters are lowercase), and uderscores are generally not used except for the m_.

There are a variety of bastardizations of this standard...

I think Zaphod's general point is: Make your code look just like other ShowEQ code from a style point of view... same tabbing, variable naming scheme, bracket placement, spacing, etc...

It just makes it easier for other devs to read the code fluidly..

--Jeeves

[Zaphod]

Originally posted by Mr. Suspicious
Well, 4th time I reply on this *sigh* 3 times my ISP went offline just before hitting submit *loooove the wonders of technoloky*

Yeah, I hate it when that happens... ;)



First off, it's only in the "rough structure stage" right now and CERTAINLY NO "patch material", perhaps it would have been 2 years ago, but not the way the code is structured today.

I understand, just giving what you asked for... or my interpretation thereof... heh... :D



- Learn more about Qt (look at the tutorials and examples in $QTDIR/doc/html/index.html) and use the Qt classes where available.
- Learn more about C++.

That's what I'm doing =) Last time I realy programmed was in Assembler on 8086s (15 years ago), nowadays I only do some php, ASP, VB stuff.

No problem, just giving a few pointers. 8086, ah, the bad old days... ASP and VB (/shiver). Qt docs are also available from http://doc.trolltech.com/2.3/



It looks like your iIconLine member of MapIcon is really a true/false value, in which case it should be a bool at the end of the structure.

Is now, will be a "line" / "noline" / "slashed line" / "dotted line" / "red line with flashy blue slashes" or whatever other lines QT allows (I'm assuming it uses the GD library)

Then the Qt::PenStyle enumerated type is what you are looking for (see http://doc.trolltech.com/2.3/qt.html#PenStyle)



- You should use the preferences system instead of a seperate text file to initialize the icon data.

True, and is planned aswell... in this stage however I wanted to stick with map.cpp for a bit and work in there before leaving the module and messing everything up *g*

Using the preference system doesn't require going out of map.cpp, just look in the Map constructor (Map::Map()) for some example usage.



- Try to stick to the method/member naming style used elsewhere in the code (we don't do polish).

"polish"? Uhm.. like "the language in Poland"? My native language is not English, so I might be missing your point here.

What high_jeeves said... Basically from your leading i before you integer variable names (from another bastard Polish variant).



- What's the point of the iIconName member of MapIcon?

Debugging purposes:

cout << "Icon for " << Icons[IconIndex].iIconName << " loaded/n"

Just to see IF it loaded and IF it loaded the correct thing in the correct Icons[section]. Is actually a direct port from a small program I use that emulates the behaviour I want *compiles in seconds compared to the 4 minute recompile of map.cpp and map.h*

Just as long as it isn't in the final version... And if you are gonna use a character string use a QString or QCString instead for the member.



- Why doesn't the TRACER filter have an icon?

To quote someone else "WTF is TRACER?", lol. No, in all fairness, all instances will be ported over in the final version (Available here Christmas 2008) to be used in the finished code. Also the PvP and Diety things and whatnot. I just needed to use a "few" as example to see if it works alright.

Last I remember TRACER was Ratt's idea... Just mentioning for completeness sake.



Thanks for the reply and taking the time to read through the code Zaphod, I realy appreciate it. If you see some other "daft" things, please let me know. *was secretly hoping someone'd go "HELL NO STOP THE INSANITY NOW! I'LL DO IT!"* anyone? :D [/B]
Glad to help. I just might jump on it at some point... ;) I just have a few other things ahead of it on the runway. All in all it isn't a bad start, especially for someone new to the code and C++.

Enjoy,
Zaphod (dohpaZ)

[Mr. Suspicious]

Thanks Zaphod, al lot of the things mentioned I have incorperated now. My the _real_ challenge is still Classes. Once filled I simply regard them as a 2 dimensional array of values. That might be where I'm offtrack tho.

If I initize Class IconData in the constructor Map::Map() I need to pass on the pointer to the Class to the appropriate functions, so it can be used in paintIcon()

To see which functions it should be passed to, I do a backtrack. Typically lookup what function calls paintIcon(), then lookup what other function calls that parent funtion and then all the way up into the Tree till I get to the constructor... but but but...

Map::paintIcons() is called in Map::paintSpawns()
Map::paintSpawns() is called in Map::PaintMap()
Map::paintMap() is called in Map::paintEvent()
Map::paintEvent() is called... nowhere...

So, I think to myself... there must be another way of passing a pointer to a grand-grand-child function, so I dive into my "C++ for Dummies", then "C++ in 21 days" (all 909 pages) and altho it does give me a lot of insight on other things I was about to stumble upon, it's everything but clear on this matter.

[high_jeeves]

I would think you would want your IconData to me a member variable of the Map class. If it is a member variable, it is accessible in all methods of the Map class (without passing it in, all member variables are in-scope for all methods of the class)

For an example, take a look at how the m_spawnShell variable is defined (in map.h) and used (in paintSpawns for example)... This is how I would recommend you do your icons as well... define them within the class in the header, initialize them in the constructor, use them in the paintIcons method..

--Jeeves

[Mr. Suspicious]

Sounds simple enough =)

define them within the class in the header

Easy enough: done

initialize them in the constructor

Aye, works like a charm

use them in the paintIcons method

Arf... here's where I go wrong somehow. The IconData is no more then a 2dimensional array of various elements (QString's and int's) So calling it via Icons[X].iIconColor would be in the line of expectation. But doesn't seem to work that way: No match for `Map::MapIcon& [int&] operator. Even creating a pointer (within the member function paintIcons() MapIcon *Icons2 = &Icons;) to point to Icons for instance gives a different adress then the actual adress Icons is located at.

I viewed the m_spawnShell variable and it appears to be a pointer (initialized in the Map class via: SpawnShell* m_spawnShell;) I'm just not getting it tho... I understand that spawnShell is pointing at it's internal functions to retrieve hardcoded values. But I don't understand how I could use that information to "mimic" it for the use I have in mind.

Perhaps I'm using the Class specifications wrong and shouldn't I view it as a 2 dimensional array but rather create 8 instances of the class and use those to retrieve the data. But that still leaves me the wrong adress. Anyway, after one week getting headaches over this I'm about to break my keyboard in two, hehe.. it sounds to easy, then, why isn't it? :D

[high_jeeves]

How about if you post what code you have here on these boards.. then perhaps the devs, and some of us other coders around here can help you out. (Just the area around what you have changed.. we dont need too much).

--Jeeves

[Mr. Suspicious]

latest diffs attached.

Quick rundown:

- placed MapIcon Class inline in Map Class.

- Create Instance of Class MapIcon named Icon in Map constructor and load the MapIcon Class there.

Hmmmm... attachement should be here, but isn't (alternative): http://www.petervandongen.com/download/mapver6.tar.gz, 3kb.

[high_jeeves]

Ok.. i took a look, and its a little tought to tell where stuff is (esp. in the header file).

1) It looks like you are actually defining the MapIcon class within the Map class... if this is correct, then move the MapIcon definition to outside (above) the Map class definition.

Like:



class MapIcon
{
<stuff>
};

class Map
{
<stuff>
};


2) In the constructor of Map (I am assuming that is where the first part of the .cpp diff is?) You have a LOCAL variable called Icons (an array of 8 MapIcon objects). This LOCAL variable overrides your CLASS variable (the one defined in the header file). So, in the constructor, you are loading 8 icons, then destroying them as they go out of scope when you exit the constructor (because they are LOCAL variables). Nowhere in the constructor are you changing the CLASS variable called Icons.

3) The CLASS variable Icons needs to be your array of 8. (or however many). Like this:



class Map
{
<stuff>
MapIcon Icons[8];
<stuff>
};


4) The way you are reading the icons is extremely inefficient. You are opening the file 8 times, and skipping forward over each one you have already read. Try something like this (this is psuedo-code):



open(file)
for (int section = 0; section < 8; section++)
{
//skip to valid front of section
read data into Icons[section];
}
close(file)


Thats all I see for now.

You keep talking about 2 dimensional arrays and classes in your posts, and I'm not entirely sure what you mean... 2 dimensional arrays are completely differnet things from classes! A class is made up of a set of member variables, and a set of methods. arrays can contain classes, and classes can contain arrays, but they arent even close to the same thing.

--Jeeves

[Zaphod]

First I'll re-emphasize some specific items from my first response:


In general, the use of <iostream.h> and <fstream.h> are deprecated in C++ (use <iostream> and <fstream> instead). Within ShowEQ you should consider using the QFile and QTextStream classes from Qt instead.
The array of icons should be made a member and initialized in the Map class constructor and not in Map::paintSpawns()
Try to stick to the method/member naming style used elsewhere in the code


Ok, now for a few additional comments:
Become more familiar with the C++, specifically the terminology and how classes work. Examples of member variables in the Map class would include "MapParameters m_param;" and "int m_frameRate;". See how they are declared in the Map class declaration in map.h, and see how they are initialized/used in the Map class methods (including the constructor Map::Map()). The big difference between a C/C++ function and a C++ method (aka member function) is that the C++ method automagically recieves a pointer (known as the "this" pointer) that points to the specific instance (this instance) of the class it is operating on and automatically dereferences its member variables for you. Notice how the Map::Map() and other methods (member functions) are able to access by name the m_param and m_frameRate member variables. So what you really wanted to do with your array of MapIcons is to put it in the Map class declaration as a member variable in map.h, ie. "MapIcon m_mapIcons[8];" and then access them just as the other member variables are used.
In this context don't think of a struct/class as an n-dimensional array of bytes. Your "MapIcon Icons[8]" local stack variable is a simple one-dimensional array of MapIcon class instances.
You really didn't need to get rid of the constants you were declaring (previously in Map::paintSpawns()), you should however have made them enumerated types (see enum {} ) in your book declared either at the top of the map.h header file or at the top of the map.cpp source file.

I hpoe these clarify things for you a little.


Originally posted by Mr. Suspicious
*was secretly hoping someone'd go "HELL NO STOP THE INSANITY NOW! I'LL DO IT!"* anyone? :D
I think I will at this point. I'll start with what you've done, clean it up, and you can see how it looks and works in the end. OK?

Enjoy,
Zaphod (dohpaZ)

[Mr. Suspicious]

Nonononono =) Please don't think I forgot about those points mentioned before Zaphod. I'm doing things step-by step and I mentioned before that classes... have me baffled somewhat. It's working now, with the things mentioned by High_Jeeves.

Now I don't have to crack my brain about the display of the icons anymore, I can step to the 2nd phase: putting and getting icons from the preference system. Sure, I'll run into Hurdles there aswell, but isn't that what learning is all about? =) I only start asking questions if I realy see no way out anymore.

Just for completeness sake, my steps as I wanted to follow them:

1. Define class and make it working (done now, altho)
2. Load Icons from preference system (and "tweak" last int usage in file to QString values for easier reading of file)
3. Interface to change icons in active memory
4. Save Icons from active memory to preference system
5. Clean up code *by using "search and replace"

Cleaning up code I partly did this afternoon to match things with what you mentioned.



You really didn't need to get rid of the constants you were declaring (previously in Map:aintSpawns()), you should however have made them enumerated types (see enum {} ) in your book declared either at the top of the map.h header file or at the top of the map.cpp source file.


They are since this morning and they were in a previous version (4 I believe), just rigged them out again, because I couldn't get the Icon Class to work (and subsequently the enums) has everything to do with my misunderstanding of how to declare consts and vars in a Class itself to use in it's memberfunctions. When I posted my last question, I assumed (and no book disproved it) that a const should be declared in the constructor and not in the header. High_Jeeves set me straight there.

I've tarred up the latest versions and I'd appreciate it if you used those instead of the previous posted diffs. Have a look at them if you like. I'm sure you can do things much faster then I will ever be able to do. But I enjoy this "mental excercise" (even when it sometimes means I'm about to pickup the PC and throw it out of the window *grins*)

If you want to take over, that's ok, escpecially if it's "standing in the way of normal progression" of ShowEQ.

Find the latest (half cleaned up and with the latest commit) sourcefiles + icons.conf (for reference) at: http://www.petervandongen.com/download/mapver7.tar.gz (33Kb)

It's a working version, missing 3/4th of the things you mentioned in your 1st reply and "lines to icons" aren't defined in file the way I want them yet.

[Zaphod]

I didn't think you forgot about those points. Just wanted to emphasize a few.

As far as continuing it, you can if you'd like. It is just a matter of I got struck on the head by inspiration from what you had done so far and so banged out some quick changes on the keyboard about 2-3 hours worth). Nothing fantastic or complete, but an improvement that includes a few new concepts.

I really don't wanna steal your thunder or mental excercise. Just a matter of inspiration combined with your "secretly hoping" and saving you from "getting headaches over this" and "break my keyboard in two".

If you wanna continue on, I'll send you my changes and you can go nuts. Just let me know.

How about you pop onto IRC at smurfette.trifocus.net port 6667 in the #showeq channel. I'm usually logged into it (as well as a couple other of the devs). I may or may not be actively paying attention, but I will notice someone mentioning my name eventually... ;)

Enjoy,
Zaphod (dohpaZ)

[Zaphod]

In case you are interested here are my changes against the current CVS as a bzip2'd diff file.

Preference loading is in but not saving, and no GUI for editing. It adds a bunch of new concepts, and capabilities, give a look see.

Edit:Here is that attachment that should have automagically appeared attached here: http://seq.sourceforge.net/attachment.php?s=&attachmentid=67

Enjoy,
Zaphod (dohpaZ)

/me ponders how much this shows how he thinks while developing... ;)

[fryfrog]

wahoo, your test did or didn't work!

[Ratt]

N/T

[Mr. Suspicious]

Tried to get hold of you on IRC Zaphod, just missed you twice =) I'll try again late this eve (my time on CET)

I looked through the diffs and my gawd... eleeeegant... much more elegant then I'd ever be able to do. I'd be honored if you'd continue on it and I'll keep my hands off it (I'm sure there'll be other things I can break and try to fix)