C++

[Panthera Leo]

Because I'm bored, and I like to code random stuff. I've already started porting Crossfire over to C++. (You'd be amazed how much classes, inheritance, and the standard libraries stream line the program.*) I tried just confining the change to something small, but the source is so built that it's easier to rewrite the code then it is to encapsulate any one part properly.

I personally think it's a good move, but then again, I'm bias on that.

When I complete the port, I'm wondering if I should fork the project or if it would be merged into the main build, and who I'd talk to about that.

I say when because the hard word was done long ago, my biggest issues are staying awake and keeping my mouse charged. If all goes well, I'll be done by Monday. If not, the end of next week, or the month.

So, Who would I talk to about a merge or fork?

*The object and map code blocks in C++ take up less space then either of them in C so far.[/i]

[Ryo]

The C or C++ question is a recurring theme on the developers' mailing list.

Historically Crossfire is plain ANSI C. Different people (including me so I'm biaised) have suggested a move to C++ (and I'd use Qt to have many free bonuses while we're at it).


But two points are to take into account:

1) the code base isn't that clean, and would require much effort to properly use C++'s features correctly. So much that a whole rethinking of the game's architecture could be in order. Merely compiling in C++ is a few hours work (been there, done that), but using advanced stuff like multithreading (ok, you can do that in C if you want) is a totally different can of worms.

2) the manpower isn't there. I'd love a C++ version with a decent code base and many bells and whistles, but the first and most important aspect of the game is the content, something not many people work on. So I feel compelled to work first on content, then the code itself.


Note that porting to C++/Qt is on my todo list, http://wiki.metalforge.net/doku.php/user:ryo:todo, but really low priority - what's the point of a good code base if no content?


I'd rather have an official C++ version, so I'm ok with importing into the official repository. But not everyone would agree, so that's another matter to discuss.



Finally, note there exists a C++ fork of Crossfire, which I won't name since the fork wasn't a smooth or nice experience. You could check with them, but if you use their code don't expect to merge it back into the official Crossfire repository.

[Rednaxela]

Warning about the aforementioned fork: They tie their codebase into perl in a very tightly coupled way that I would consider poorly maintainable. Also some individuals in there are widely considered to be abrasive.

Anyway, if you feel you want to try to port CF into C++, feel free to. Consider what Ryo has noted about it. I'd suggest to perhaps drop by the #crossfire IRC channel on Freenode.net if you have any questions or want to talk about it. A fair number of people (such as myself) don't keep track of the forums as much as IRC really.

I'd agree with getting it merged in the main build if the specifics of the port look good.

[Panthera Leo]

Sweet, motivation!

I'll looking to the IRC.

As for conforming to "specs". What exactly are the specifications other then maintaining the current server/client message protocol(s)? I don't see any coding standards in my SVN of the server source.

[Rednaxela]

Well, this probably the document you're looking for.

Of course much of it doesn't apply to C++ and there are aspects of C++ that are not covered by it as well. My advice, at least as far as the likes of function/class naming would be to keep in the same vein as the standard libraries, or if you use QT or Boost C++ libraries (I personally think using one of the two would be a good idea) matching naming conventions of those. That's my personal view anyway.

[mwedel]

In terms of availability, I think the first step would be to check it in as a new top level dir. EG, right now, we have client, server, maps, arch, jxclient, etc. So putting in a server_c++ would make sense (I'm presuming SVN will allow + in project names).

In that way, it lets the other developers look at it and get a feel, make suggestions, etc.

Like Ryo, I'm in the camp the major rewrites of the code isn't a priority, but rather features that improve the player experience directly are. That may mean code changes, but in the end, the players probably don't care if the server is written in C++, java, C, or assembly.

The hardest part of any rewrite is keeping it in sync with latest code and making it feature complete.

If it's not feature complete, then folks are less likely to use it, and also not as likely to keep it up to date with respect to new features. Realistically, maintaining multiple versions is extra work for everyone involved, so it has to get to a point where it replaces what is currently in use fairly quickly, or there is a risk it will get too far out of date and become unmaintained.

Another disadvantage of big changes is that for those of us familiar with the code, it basically becomes a relearning experience to figure out where everything is/how everything works.

[Panthera Leo]

If you don't want it, then don't accept it. I'm jazzed about working on a project, but I'm not going to go home crying. Thinking C++ is just how think, and it's why I even had the idea to look into the source. I was look at the source to figure out how diseases and spells worked for my own pet map, and realised "Hey, I could improve that."

I wrote that because I was bored ... for a script ... in a day ... for a script ...

Map.hpp

Code: Select all

/*
 * CFAid.h
 *
 *  Created on: Feb 13, 2010
 *      Author: Stephen Wooddell
 *
 *  Copyright 2010 Stephen Wooddell
 *
 *  CFAid is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 3 of the License, or
 *  (at your option) any later version.
 *
 *  ComCal is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with ComCal.  If not, see <http://www.gnu.org/licenses/>.
 */

#ifndef CFAID_H_
#define CFAID_H_

#include <string>
#include <sstream>
#include <vector>
#include <list>
#include <iostream>

//-------------------------------------Variables------------------------------------
const std::string Black 					= "draw 1 ";
const std::string DarkBlue 					= "draw 2 ";
const std::string Red 						= "draw 3 ";
const std::string DarkYellow				= "draw 4 ";
const std::string LightBlue					= "draw 5 ";
const std::string Orange					= "draw 6 ";
const std::string LightGreen				= "draw 7 ";
const std::string VeryLightGreen			= "draw 8 ";
const std::string Grey						= "draw 9 ";
const std::string Brown						= "draw 10 ";
const std::string Yellow					= "draw 11 ";
const std::string Tan						= "draw 12 ";

const unsigned int HoldForTick			= 0x00000001;
const unsigned int HoldForHp			= 0x00000002;
const unsigned int HoldForMaxHp			= 0x00000004;
const unsigned int HoldForSp			= 0x00000008;
const unsigned int HoldForMaxSp			= 0x00000010;
const unsigned int HoldForGrace			= 0x00000020;
const unsigned int HoldForMaxGrace		= 0x00000040;
const unsigned int HoldForFood			= 0x00000080;

const unsigned int HoldForMonitor		= 0x00000100;
const unsigned int HoldForMessage		= 0x00000200;
const unsigned int HoldForItemRequest	= 0x00000400;
const unsigned int UserHold				= 0x00000800;
const unsigned int SkipTick				= 0x00001000;
//----------------------------------------------------------------------------------
//
//--------------------------------------Classes-------------------------------------
class CFAid
{

	public:
		struct Stats
		{
			unsigned int Hp;
			unsigned int MaxHp;
			unsigned int Sp;
			unsigned int MaxSp;
			long Grace;
			long MaxGrace;
			unsigned int Food;
		};

		class Functionoid;
		typedef void (*FncPtr) ( Functionoid&
			, CFAid&
			, const std::vector<std::string>&);
		typedef std::list<Functionoid>::iterator Iterator;

	protected:
		Stats ThisTick, LastTick;
		std::list<Functionoid> FunctionList;

	public:
		//Iterates though the function list and removes the given hold.
		void RemoveHolds (const unsigned int&);
		//Adds and returns a iterator to that function.
		Iterator AddFunctionoid (const Functionoid&);
		//Adds and returns a iterator to that function.
		void RemoveFunctionoid (const Iterator&);

		const Stats& GetThisTick () const;
		const Stats& GetLastTick () const;

		//Turns on and requests the needed watches.
		void Init() const;
		//Possesses a update, removes any removes, and executes any function /w none.
		void ProcessUpdate (const std::vector<std::string>&);
};

class CFAid::Functionoid
{
	public:
		unsigned int Holds;
		FncPtr FunctionCall;

		Functionoid (const FncPtr& Fnc , const unsigned int& Hlds = 0)
			:Holds(Hlds),FunctionCall(Fnc){}
		//Sets a hold bit for a functionoid.
		inline void SetHold (const unsigned int& Flags) { Holds |= Flags; }
		//Removes a hold bit for a functionoid.
		inline void RemoveHold (const unsigned int& Flags) { Holds &= ~Flags; }
		//Tests if a function bit is on.
		inline bool GetHold (const unsigned int& Flags) { return Holds & Flags; }
		//Returns a constant reference to the function bits.
		inline const unsigned int& GetHolds() { return Holds; }
};

//Iterates though the function list and removes the given hold.
void CFAid::RemoveHolds (const unsigned int& Flags)
{
	for(Iterator It = FunctionList.begin() ; It != FunctionList.end() ; It++)
		It -> RemoveHold(Flags);
}

//Adds and returns a iterator to that function.
CFAid::Iterator CFAid::AddFunctionoid (const Functionoid& Fn)
	{return FunctionList.insert( FunctionList.end() , Fn );}

//Adds and returns a iterator to that function.
void CFAid::RemoveFunctionoid (const Iterator& It)
	{FunctionList.erase( It );}

inline const CFAid::Stats& CFAid::GetThisTick () const {return ThisTick;}
inline const CFAid::Stats& CFAid::GetLastTick () const {return LastTick;};

//Turns on and requests the needed watches.
inline void CFAid::Init() const
{
	std::cout << "request stat hp\n";
	std::cout << "watch stats\n";
	std::cout << "watch tick\n";

	std::cout.flush();
}
//----------------------------------------------------------------------------------

//-------------------------------------Functions------------------------------------
//Searches a string and divides each part divided by white space into elements.
std::vector<std::string> StringToVector(const std::string& String)
{
	std::vector<std::string> Return;

	std::string StingFragment;

	for (unsigned int i=0; i < String.length(); i++)
	{
		if ( String[i] > ' ' ){ StingFragment += String[i]; }
		else
		{
			if (StingFragment.length() > 0)
				Return.push_back(StingFragment);
			StingFragment = "";
		}
	}

	if (StingFragment.length() > 0) {Return.push_back(StingFragment);}

	return Return;
}

//Possesses a update, removes any removes, and executes any function /w none.
void CFAid::ProcessUpdate (const std::vector<std::string>& VectorString)
{
	if( VectorString[0] == "watch")
	{
		if( VectorString[1] == "tick")
		{
			LastTick = ThisTick;
			for(Iterator It = FunctionList.begin() ; It != FunctionList.end() ; It++)
			{
				if ( (It -> Holds) & SkipTick )
				{
					It -> RemoveHold(SkipTick);
					It -> SetHold(HoldForTick);
				}
				else
					{It -> RemoveHold(HoldForTick);}
			}
		}
		else if( VectorString[1] == "stats")
		{
			if( VectorString[2] == "hp" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.Hp;
				RemoveHolds(HoldForHp);
			}
			else if( VectorString[2] == "maxhp" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.MaxHp;
				RemoveHolds(HoldForMaxHp);
			}
			else if( VectorString[2] == "sp" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.Sp;
				RemoveHolds(HoldForSp);
			}
			else if( VectorString[2] == "maxsp" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.MaxSp;
				RemoveHolds(HoldForMaxSp);
			}
			else if( VectorString[2] == "grace" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.Grace;
				RemoveHolds(HoldForGrace);
			}
			else if( VectorString[2] == "maxgrace" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.MaxGrace;
				RemoveHolds(HoldForMaxGrace);
			}
			else if( VectorString[2] == "food" )
			{
				std::stringstream( VectorString[3] ) >> ThisTick.Food;
				RemoveHolds(HoldForFood);
			}
		}
	}
	else if( VectorString[0] == "request" )
	{
		if( VectorString[1] == "items" )
			{ RemoveHolds(HoldForItemRequest); }
		else if( VectorString[1] + VectorString[2] == ( "stat" "hp" ) )
		{
			std::stringstream( VectorString[3] ) >> ThisTick.Hp;
			std::stringstream( VectorString[4] ) >> ThisTick.MaxHp;
			std::stringstream( VectorString[5] ) >> ThisTick.Sp;
			std::stringstream( VectorString[6] ) >> ThisTick.MaxSp;
			std::stringstream( VectorString[7] ) >> ThisTick.Grace;
			std::stringstream( VectorString[8] ) >> ThisTick.MaxGrace;
			std::stringstream( VectorString[9] ) >> ThisTick.Food;

			RemoveHolds(HoldForHp
				| HoldForMaxHp
				| HoldForSp
				| HoldForMaxSp
				| HoldForGrace
				| HoldForMaxGrace
				| HoldForFood);
		}
	}
	else if ( VectorString[0] == "monitor" )
		{ RemoveHolds(HoldForMonitor); }
	else if ( VectorString[0] == "scripttell" )
		{ RemoveHolds(HoldForMessage); }
	for(Iterator It = FunctionList.begin() ; It != FunctionList.end() ; It++)
		if ( It -> GetHolds() == 0 )
		{
			It -> FunctionCall ( *It , *this , VectorString);
			std::cout.flush();
		}
}

//----------------------------------------------------------------------------------

#endif /* CFAID_H_ */

If recoding is a issue for you, it's something I'm trying to avoid. I'd love to work with you as I can, but I'm flying mostly by wire.

I've got at least two idea I'd like to add. Maps that are parallel to the main maps that can be travelled between. (Though half of that idea is ripped from Masters of Magic.) "Warp space" that links into the main map at intervals instead of a 1:1 ratio. A new school of magic that works off the inscription skill to fixed runes that cost in coin or hem, and use "ambient power" to charge period or constant affects over feeding off spell points or grace. (Basically a 'glowing crystal' with a recharge value, a spell, and a "fire" interval.)

I'm just OCD about getting the code in a sane order first. Sure, the code as is could be improved and made to be less "patch-work", but it's a solid code. I'm not reinventing the wheel and slapping the name "crossfire" on it. I'm just working out how to put everything into C++ style classes, insulate them from future changes, and just porting the existing code over to C++. There is nothing "new" to spend much time on yet.

Edit2: removed dated and ugly code.

The only signifcant differances (aside from being a C++ rewrite), nothing is publically accesable anymore. That's not to force a rewrite, but make sure if something is rewritten it will always have the same interface.

Well, that and the copy-right. As no one else has added to the code yet, no one else has a claim to the file. As it's a new thing, I thought a new copy-right was needed. It's not a large issue to me how it works out, just as long as it works out.

[Panthera Leo]

First I should say thank you, I've been unfairly critical of the current code base and haven't given a proper thank you for the writing Crossfire in the first place! It is a job well done. I belittled the project by saying I could be done so quickly. I should reiterate how I only think that because most of the work was done long ago by some clever people. Until I submit the code, somewhere, and someone else adds to it I don't claim a copy-right to say the work is all mine. I do that just to ensure someone has a solid claim to it to offer it under the GPL. I gladly give credit to the Crossfire team for their work!

Now I have three question about the code that no one in the IRC was able to answer:

• First, To keep things "1(1)" I've added the "map layer" array to the map class I'm writing. I did so because on examining 'map.h' I saw the array and the macros referencing it. I assumed it was a requirement, and coded it. Now that I'm almost done with it I'm looking over all the code in detail to make sure what I did lines up with what should be. I can not understand what the layers array is used for beyond 'memcpy' is currently taken care differently in my build with class inheritance:
  
  Can someone help me?
• Second, On the issue of map loading taking so much time. I'd like to change the format, or convert to byte codes. However, as I'm frequently reminded, and rightly so, that would not be "1(1)". I'd like feed back on add a "sub-system" to catch map information in binary format. Not a idea solution, but a server could write a binary version of a map file(s) along side the current map files for quicker loading, and easy indexing. On exit, or on command the files could be reset to their current format for editing or other use.
  
  Feedback?
• Third, could I include SDL in the rebuild? It would include it's own timer that would work in a "1(1)" manner. Would allow for future multi-threading and 2D GUIs. Failing that, may I defer to "ISO C99: 7.18 Integer types <stdint.h>" to handle the uint and sint types?
  
  Feedback?

Sadly things are not on-schedule as I've had to trouble shoot many inane issues with my IDE and the GCC. I should a fine version of the my map class ready for review a few hours after I receive a answer though.

[Ragnor]

First, To keep things "1(1)" I've added the "map layer" array to the map class I'm writing. I did so because on examining 'map.h' I saw the array and the macros referencing it. I assumed it was a requirement, and coded it. Now that I'm almost done with it I'm looking over all the code in detail to make sure what I did lines up with what should be. I can not understand what the layers array is used for beyond 'memcpy' is currently taken care differently in my build with class inheritance:

Which "map layer" variable (or field?) are you talking of?

Second, On the issue of map loading taking so much time. I'd like to change the format, or convert to byte codes. However, as I'm frequently reminded, and rightly so, that would not be "1(1)". I'd like feed back on add a "sub-system" to catch map information in binary format. Not a idea solution, but a server could write a binary version of a map file(s) along side the current map files for quicker loading, and easy indexing. On exit, or on command the files could be reset to their current format for editing or other use.

The real problem is not that map loading is too slow but that loading maps containing a huge number of objects affects unrelated players. Currently the server does all processing in one thread. Therefore any action that takes more than a fraction of a second (for example loading a huge map file) stalls all other actions (for example movement of other players).

A conversion to binary format might speed up map loading. But this speed up would be just O(1) (i.e., having a constant factor related to the number of objects within the map). Therefore the initial problem remains, no matter how fast a binary format could be loaded: players would just adapt and stuff more items into their apartments until the lag again gets noticeable.

OTOH, converting all map files into a binary format (IMO) has some severe drawbacks: all existing tools have to be adapted; simple examination or modification of map files (using less, grep, vi, sed, etc.) does not work anymore; corrupted map files (for whatever reason) cannot be fixed easily; the server has much less possibilities to detect corrupted map files, so chances are that problems will not be noticed for a long time.

Storing the binary map files just as cached versions of the original plain text files also has drawbacks: checking and converting all map files at server startup would take much time (and would require write access to map files from the user starting the server); opening swapped map files with the editor would not work anymore; two map file formats would have to be maintained.

IMO a "proper" fix could include preventing too many items per map and/or per tile. In-game explanation: there is just not more space for the items available.

Another possible fix could be to make the server multi-threaded. In such a server, loading a huge map would still cause lags. But the lags would affect only the player(s) related to this map. (But as this would definitely cause many new issues (due to race conditions), I'd vote against this change.)

That said, I'd strongly vote against a binary map file format.

Third, could I include SDL in the rebuild? It would include it's own timer that would work in a "1(1)" manner. Would allow for future multi-threading and 2D GUIs.

Crossfire already needs quite a few libraries. This makes it inconvenient to just "download and compile" it. Adding yet another library would not help this situation.

Generally speaking: adding a new library is ok but should provide much improvements. Just "would allow for future ..." is not a good argument to me: Crossfire already consists of zillions of features that someone did add because "it could be useful sometime". Most of these features are not used in-game.

(All "IMO", of course.)

Failing that, may I defer to "ISO C99: 7.18 Integer types <stdint.h>" to handle the uint and sint types?

AFAIK Crossfire code should be C89 (not C99). I think part of it is because no C99 Windows based compiler exists. (Ignoring gcc which I think nobody uses for compiling a Crossfire server on Windows.)

Generally speaking: I would not object switching to C99. Especially since quite often commits already add C99 specific code such as mixing declarations and statements, using // comments, or calling C99-only functions such as snprintf().

[Panthera Leo]

Which "map layer" variable (or field?) are you talking of?

edit: Question answered, Thank you Ragnor.


-----------------------------------------------------------------


Your right about the binary it's a temporary fix until multi-threading is enabled or someone piles things up in their apartment to bog things down again. It's the best idea I have. The second is just loading a file pointer, a object's name, and picture. The delaying loading the rest of the object until the rest of the information is needed. However, I can never put such a theory into use before.

No matter what I do to speed thing up, it's only a matter of time until someone bogs that method down too. Even if I or someone else mutli-threads the program, it's still only a matter of time before the drive read-times become a issue or until they grow the file size to 20meg because they can load more now? Then what? Require a mirrored RAID set-up?

The only absolute way to stop long load times is to limit what is loaded. Automatically place object over a set number in a container that isn't loaded until accessed?


-----------------------------------------------------------------

corrupted map files (for whatever reason) cannot be fixed easily; the server has much less possibilities to detect corrupted map files,

That's true and it's not. Yes, it's beyond the scope of the target uses to debug a binary file. (edit) The only code difference would be telling the object that loads and unloads files to dump information in the raw binary instead of converting to and from plain text, removing the white space, and truncating the keywords to one letter.

Crossfire already needs quite a few libraries. This makes it inconvenient to just "download and compile" it. Adding yet another library would not help this situation.

This is true. I'm spoiled with everything in packages for me with my Linux set-up, such dependences have been taken care of for me for ... years. Though SDL is used as a shared object/DLL file. It's why I consider it.

It's been doing a number of functions internally for a considerable amount of time. ("first releasing it in early 1998"). True, this would force windows users to download a .dll file. However, it takes more time to tell you that's a easy download then it does to find and download it. It could easily be added to pre-compiled versions. (145K zip.)

http://www.libsdl.org/download-1.2.php

Are you sure about that? Are you sure you will not support it? The base line SDL does considerably more in one package, the timer and typedefs are just what I'd use it for currently. I will abide either way, but are you sure?

AFAIK Crossfire code should be C89 (not C99).

Eclipse is a multi-language ,iirc international, java IDE that can use the GCC, but very well. (Did I forget to say that it's free and awesome?) Lazy windows users!

Edit: As promised, I've something to submit that I'd like to see if this is what you want. How would I do that?