Jump to content
  • Advertisement
Sign in to follow this  
cozzie

Ugly, can this be done easier?

This topic is 1321 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi all,

Just of curiosity, how would you achieve the same goal as I do with the code below?

It feels a bit 'clumsy', but I didn't find a way to do this easier/ less code repetition.

		D3DCOLOR selectedCol = D3DCOLOR_XRGB(255, 255, 0);
		D3DCOLOR defCol = D3DCOLOR_XRGB(255, 255, 255);
		D3DCOLOR tempCol;

		switch(mCurrentMenu)
		{
			case MENU_MAIN:
			{
				// ITEMS
				if(mGameStarted)
				{
					if(mCurrentMenuItem == 0) tempCol = selectedCol; else tempCol = defCol;
					mD3d.mD3dFont.PrintLarge("Resume game", startMenuX, startMenuY + 1 * offsetMenuItem, tempCol);	// 0
				}

				if(mCurrentMenuItem == 1) tempCol = selectedCol; else tempCol = defCol;
				mD3d.mD3dFont.PrintLarge("New game", startMenuX, startMenuY + 2 * offsetMenuItem, tempCol);			// 1
				if(mCurrentMenuItem == 2) tempCol = selectedCol; else tempCol = defCol;
				mD3d.mD3dFont.PrintLarge("Options", startMenuX, startMenuY + 3 * offsetMenuItem, tempCol);			// 2
				if(mCurrentMenuItem == 3) tempCol = selectedCol; else tempCol = defCol;
				mD3d.mD3dFont.PrintLarge("Leaderboard", startMenuX, startMenuY + 4 * offsetMenuItem, tempCol);		// 3
				if(mCurrentMenuItem == 4) tempCol = selectedCol; else tempCol = defCol;
				mD3d.mD3dFont.PrintLarge("Quit game", startMenuX, startMenuY + 5 * offsetMenuItem, tempCol);		// 4

				break;
			}
		}

Note; don't mind the switch has just one case for now, will be more later on

Share this post


Link to post
Share on other sites
Advertisement
You could get rid of the if(mCurrentMenuItem == x) stuff by using ternary operators like this:
mD3d.mD3dFont.PrintLarge("Some text", startMenuX, startMenuY + x * offsetMenuItem, mCurrentMenuItem == x ? selectedCol : defCol);
And you can also reduce the amount of code by adding all your string into a list and then iterate over it, so you only need the print code once.

Share this post


Link to post
Share on other sites

To demonstrate, something like

static const D3DCOLOR selectedCol = D3DCOLOR_XRGB(255, 255, 0);
static const D3DCOLOR defCol = D3DCOLOR_XRGB(255, 255, 255);

switch(mCurrentMenu)
{
	case MENU_MAIN:
	{
		static const char* const items[]
			= {
				"Resume game",
				"New game",
				"Options",
				"Leaderboard",
				"Quit game",
				nullptr
			};
		uint idxItem = mGameStarted ? 0 : 1;
		while (items[idxItem]!=nullptr) {
			mD3d.mD3dFont.PrintLarge(items[idxItem], startMenuX, startMenuY + idxItem * offsetMenuItem, mCurrentMenuItem==idxItem ? selectedCol : defCol);
                        idxItem++;
		}
	} break;
}

Because I don't know what type the first parameter of PrintLarge actually expects, I've used (undesired) char* here for simplicity.

Share this post


Link to post
Share on other sites

As a further enhancement you should consider making the menus data driven. e.g. the position and text of each entry should be within a configuration file which can be adjusted without a recompile. This usually makes for more readable and reusable code.

Share this post


Link to post
Share on other sites

As a further enhancement you should consider making the menus data driven. e.g. the position and text of each entry should be within a configuration file which can be adjusted without a recompile. This usually makes for more readable and reusable code.

class Menu
{
static const char* items[NUM_MENU_ITEMS];
// ..

public:
const char* ItemString (const int nID);
};

Share this post


Link to post
Share on other sites

thanks, will think about that too, might be useful especially if/ when I have more menu's

Share this post


Link to post
Share on other sites

Much better:

				for(size_t id=startFrom;id<mMainMenu.GetNrItems();++id)
				{
					D3DCOLOR myCol = D3DCOLOR_XRGB(mMainMenu.GetColor(id).r, mMainMenu.GetColor(id).g, mMainMenu.GetColor(id).b);
					mD3d.mD3dFont.PrintLarge(mMainMenu.GetItemText(id).c_str(), startMenuX, startMenuY + (id+1) * offsetMenuItem, myCol);
				}

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!