Cracktro :)

The Partridge Family were neither partridges nor a family. Discuss.
Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Cracktro :)

Post by Byteraver » October 8th, 2019, 9:35 pm

Hi Guys

I had some fun creating a little so called cracktro for your pleasure, it most prominent feature being the .mod replay routine in it (it has music!). Enjoy! Source on https://github.com/TheRealByteraver as always, it ain't cleaned up though as I really hacked this together in a few days. I attached a screenshot so you'll have an idea what it's about. Unpack the attached file and double click the "modplay.exe" file to start the replay.
screenshot.png
screenshot.png (68.46 KiB) Viewed 8684 times
Peace out
- Erlando
Attachments
Intro.rar
(526.73 KiB) Downloaded 195 times

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 9th, 2019, 8:33 am

Holy balls, the scrolling text on the bottom goes on for days. Nice effects and nice choice on the music.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 9th, 2019, 5:48 pm

haha, the full text is in the file https://github.com/TheRealByteraver/RTS ... ne/intro.h if you don't want to wait for the scroller to finish ;). The major accomplishment here is the music replay routine, which took years to make. My first attempt was in Borland Pascal 7 and assembler, in the late nineties. At that time you still needed to write your own soundcard drivers, control the DMA and interrupt controllers directly etc. It could play .mod files only, on the legendary GRAVIS Ultrasound and the shitty SoundBlaster family of soundcards.

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: Cracktro :)

Post by chili » October 16th, 2019, 4:20 am

Oh my, this is quite FABULOUS. Thanks for sharing :D

You could probably make your mod player play streaming at runtime via XAudio2, would be a sweet little project.
Chili

User avatar
krautersuppe
Posts: 91
Joined: September 14th, 2015, 10:58 pm
Location: Istanbul

Re: Cracktro :)

Post by krautersuppe » October 16th, 2019, 5:40 am

2 fucking good...
I hope this will get featured in your game or maybe added to project twin or something.
DSU
Discord: dsu1, GitHub: https://github.com/DSpUz

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 21st, 2019, 6:40 pm

Hi :) Glad you like it. It is actually the intro of the game, the rest of the game is just not accessible in this version.
I succeeded in making it take proper use of the WAVE_MAPPER device this WE, thanks to David Overton for his great tutorial on the windows Wave Mapper functions (http://www.planet-source-code.com/vb/sc ... 2&lngWId=3).
So the mod/s3m/... player is now happy with about 3 MB of ram and no longer stops playing after 16'. :mrgreen:

I will have a look at using XAudio2, some guy on youtube made a tutorial about it :lol:

--> https://www.youtube.com/watch?v=T51Eqbbald4

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 23rd, 2019, 5:02 pm

Looking through the code, you're right it is a mess :)

You must have either worked on this months apart or had some copy/paste fun with code snippets around the internet. There's a lot of mixing between C and C++ code.

An example:
In the IniFile::getKeyValue( std::string, std::string, std::string ) function you use two different methods for comparing strings
line 186: if( stringList_[ currentRow_ ].compare( sectionStr ) != 0 ) continue;
line 196: if( strcmp( keyStr.c_str(), strBuf ) == 0 ) // found the key!

Since you are only looking for inequality, stringList_[ currentRow_ ] != sectionStr would have done just fine. The second one using the C library strcmp I see why it's used since strBuf is a char array, but strBuf could have been an std::string. You also use a lot of pointer arithmetic instead of using iterators, while debugging using iterators can be safer since they provide bounds checking.

Some of the for loops could be converted to STL algorithm calls.
Original code:

Code: Select all

int IniFile::readFile( const std::string& filename )
{
	// keep track of the filename for debugging purposes:
	iniFilename_ = filename;
	// start with a clean empty string list
	stringList_.clear();
	iniFileLoaded_ = false;
	// Open the ini file
	std::ifstream iniFile( filename.c_str() );
	if ( !iniFile.is_open() ) return -1;
	// read the whole file into memory
	std::stringstream rawData;
	rawData << iniFile.rdbuf();
	iniFile.close();
	// read the list line by line and remove comments, whitespace and illegal
	// commands
	char strBuf[INIFILE_MAX_LINE_LENGTH];
	bool sectionFound = false;
	for ( ; !rawData.eof(); )
	{
		// this function only works if the line it reads is shorter than 
		// MAX_LINE_LENGTH. If not, the whole function is screwed.
		rawData.getline( strBuf,INIFILE_MAX_LINE_LENGTH );
		deleteWhiteSpace( strBuf );
		// strip any possible comments from the line and check if an equal sign
		// is present
		bool equalSignFound = false;
		bool bracketOpenFound = false;
		bool bracketClosedFound = false;
		for ( char *c = strBuf; *c != '\0'; c++ )
		{
			if ( *c == '[' ) bracketOpenFound = true;
			else if ( *c == ']' ) bracketClosedFound = true;
			else if ( *c == '=' ) equalSignFound = true;
			else if ( *c == INIFILE_COMMENT_CHAR )
			{
				*c = '\0';
				break;
			}
		}
		if ( strlen( strBuf ) == 0 ) continue;
		if ( equalSignFound )
		{
			// ini file should start with a [section]
			if ( (!sectionFound) ) continue;
		} 
		else
		{
			if ( !(bracketOpenFound && bracketClosedFound) ) continue;
			else 
			{
				if ( (strBuf[0] != '[') || (strBuf[strlen( strBuf ) - 1] != ']') )
					continue;
				sectionFound = true;
        		}
		}
		// put everything in upper case for easy comparing later on
        	std::string str( strBuf );
        	std::transform( str.begin(),str.end(),str.begin(),::toupper );
        	// push the data into the buffer
		stringList_.push_back( str );
	}
	rewind();
	iniFileLoaded_ = true;
	return 0;
}
With algorithms:

Code: Select all

void IniFile::readFile( const std::string& filename )
{
	// keep track of the filename for debugging purposes:
	iniFilename_ = filename;
	
	// start with a clean empty string list
	stringList_ = std::vector<std::string>{};
	iniFileLoaded_ = false;
	
	// Open the ini file
	std::ifstream iniFile( filename.c_str() );
	if( !iniFile.is_open() ) 
		throw std::runtime_error( filename + " was not found." );
	
	// read the whole file into memory
	auto rawData = std::stringstream{} << iniFile.rdbuf();
	iniFile.close();
	
	// read the list line by line and remove comments, whitespace and illegal
	// commands
	for ( bool sectionFound = false; !rawData.eof(); )
	{
		auto strBuf = std::string();
		std::getline( rawData, strBuf );
		strBuf = deleteWhiteSpace( strBuf );
	
		// skip comments
		if( strBuf.find( INIFILE_COMMENT_CHAR ) != std::string::npos )
			continue;

		auto const equalSignFound = strBuf.find( '=' ) != std::string::npos;
		auto const bracketOpenFound = strBuf.find( '[' ) != std::string::npos;
		auto const bracketClosedFound = strBuf.find( ']' ) != std::string::npos;
		
		// ini file should start with a [section]
		if ( equalSignFound ) 
		{
			if( !sectionFound ) continue;
		}
		else
		{
			if( !( bracketOpenFound && bracketClosedFound ) ) continue;

			sectionFound = true;
		}
	
		// put everything in upper case for easy comparing later on
		std::transform( strBuf.begin(), strBuf.end(), strBuf.begin(), ::toupper );
	
		// push the data into the buffer
		stringList_.push_back( std::move( strBuf ) );
	}

	iniFileLoaded_ = true;
	rewind();
}
Other than using the std::string::find function, I opted to throw an exception instead of returning -1. Throwing exceptions from what I understand if fine as long as it's not in a hot path, and loading INI files at the beginning is not considered a hot path. You aren't always checking the return values of these functions so errors can still slip through the cracks.

You have a lot of work put into the code so far, so I don't expect a refactoring of it. Going forward, I'd suggest using more of the language features and the STL library.
  • C++17 guarantees in most cases copy and move elision for return values. This means you can stop using in/out parameters.
  • C++17 also has std::optional for cases where you can't always return a value, it's like returning a pointer, but with value semantics so there isn't a heap allocation.
  • Iterators are safer to use than pointers, but may be a little slower during Debug.
  • Prefer using the STL algorithms over raw loops when possible.
  • Prefer range based for loops over indexed based loops if algorithms won't fit the scenario.
  • Personal opinion, prefer index based loops over pointer or even iterator arithmetic, just makes the code easier to follow.
  • Prefer more descriptive names for variables, even in function bodies. *s++ = *c++ is not very clear and is pretty ugly.
  • Space out your code every few lines to make the code more readable and prevent eye-strain.
  • Separate your code according to what it does. The chili framework has an Game::UpdateModel() function and a Game::ComposeFrame() function to indicate where code should go. The UpdateModel() function should be used to update all your objects, and the ComposeFrame() function is used to draw them. Almost all of your code updates and draws in the same functions. This leads to the next suggestion...const functions.
  • For functions that DO NOT or SHOULD NOT modify member variables, declare the functions as const. Draw functions in almost if not all cases should not modify members, so they should be declared as const.
  • Functions and classes should have one responsibility, prefer breaking up functions or classes that do more than what the name of the function or class suggests. The Sprite and Graphics classes for instance have some non-generic draw functions that would be specific to a user interface. These functions could be moved to more specific classes like a UIButton class as an example.
  • Prefer using constructors over Init() functions. Using exceptions here is almost a necessity since constructors don't have return values, but using constructors is more efficient.
  • Since the framework provides a Color class, you don't have to do all the bit shifting ahead of time, it's already done for you in the Color constructors.
  • Going back to const, local variables can quite often be declared const. This helps the compiler optimize some code, prevents accidental changes and in some cases may help you rethink the structure of your code making it easier to follow.
  • Prefer std::unique_ptr over new/delete allocations
  • Prefer std::string over const char* in most cases. While the C library has all the same functionality as std::string, using std::string makes the code easier to read and in a lot of cases, you seem to be converting the const char* strings to std::string anyway. In some cases, if the string is small enough no heap allocations are done because of Small Buffer Optimization. While this isn't a standard, it's pretty common among library implementers to implement a stack based C-style array for smaller strings. The size of the std::string class in MSVC is 40 bytes, so assuming they use 16 bytes to store the size and capacity of the container, that leaves 24 bytes for SBO.
  • Use move semantics when possible for containers or classes that manage resources. The Sprite class for instance is a good example. You have a copy constructor and copy assignment operator, adding a move constructor and move assignment operator would be beneficial. This combined with using the constructors over init/load functions can help reduce the amount of maintenance code in class.
  • The Sprite and Graphics class store a Font*, which requires checks. It would probably be better to just pass a Font const& to functions needing the font. This will help make the code more readable by not having those checks and error conditions in the functions. It will be more efficient for the same reasons especially since you can't pass a null reference. This kind of goes along with separating the responsibilities of classes.
There is a lot of code duplication between the Sprite and Graphics classes, and I understand why since you may want to procedurally generate a sprite. One way to handle this would be to use inheritance where your base class would have all the draw functions and your derived classes ( Sprite and Graphics ) would implement the putPixel function(s). Another way would be to use templates and a common interface class. Some graphics libraries ( Direct2D for instance ) would call this a RenderTarget class. All the draw functions would be in the template RenderTarget class and the putPixel functions would be passed on to the class used as the template parameter.

Code: Select all

// Pseudo code
class Sprite{
public:
	void putPixel( int x, int y, Color color );
};

class Graphics{
public:
	void putPixel( int x, int y, Color color );
};

template<typename T> class RenderTarget{
public:
	RenderTarget( T& _canvas )noexcept
		:
	m_canvas( _canvas )
	{}

	void putPixel( int x, int y, Color color ){
		m_canvas.putPixel( x, y, color );
	}
	void drawBlock( int x1, int y1, int x2, int y2, Color color )noexcept{
		for ( int y = y1; y <= y2; y++ )
			for ( int x = x1; x <= x2; x++ )
				putPixel( x,y,color );
	}
private:
	T& m_canvas;
};

RenderTarget<Sprite> tntLogo;
RenderTarget<Graphics> gfx;
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 23rd, 2019, 8:11 pm

These are suggestions, take them or leave them. Your program works fine so far and it's better to get the project done I suppose than to worry about the little things. Chili's latest videos on HW3D kind of covers design. Do it as dirty as you want to get the features working, THEN go back and make it look better and act safer.

It's easier if done in iterations or steps though. Trying to make the entire project then refactoring would be a bad idea. Add a feature, debug, then refactor, add a feature, debug, then refactor.

If I find/think of anything else I'll post here. Again, nice work.

Oh, prefer constexpr over macros. I don't think there is any change in performance, and the only benefit as far as I can tell is constant expressions are typed and during debug, constexpr variables are show in intellisense when you hover your mouse over them.
#define SOME_GLOBAL_FLOAT_VALUE 2.0f // the f at the end means float
constexpr float SOME_GLOBAL_FLOAT_VALUE = 2.0f; // declared explicitly as float, but could also use auto.
constexpr auto SOME_GLOBAL_FLOAT_VALUE = 2.f; // This tells the compiler to deduce the type

With C++17 enabled in your project settings ( VS2017 or higher ), you can also add constexpr to the beginning of almost all your free and member functions. This means in some cases you'd be able to do some math during compile time. There are other reasons to declare them as constexpr, but a little complicated and I don't fully understand all of them.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 24th, 2019, 5:37 pm

Can't believe I missed this one:

Code: Select all

            optionsList_._Pop_back_n( optionsList_.size() - maxNrListItems );
Functions in the standard library that begin with an underscore should never be used. These are library specific and may change with different versions of the library. In fact, it apparently did or this function has been moved to private in later versions of the library than you originally used. When I updated the SDK to use the Win10 version of the STL this function was no longer accessible.

The correct way is to use the std::vector::erase(firstIterator, lastIterator) member function that takes a range.

Code: Select all

            optionsList_.erase( optionsList_.end() - maxNrListItems, optionsList_.end() );
Also, just found some nice and useful notes you left yourself about inheritance and smart pointers. There is one note there that is semi-inaccurate.
- As arguments to a function, pass unique pointers by reference, unless you need to
- transfer ownership, then transfer by value
This is not what chili said in his video ( awesome you put a link to the YT video ). The only time you need to pass the unique_ptr by reference or const reference is for algorithms, otherwise dereference the unique_ptr to pass the underlying object by reference or const reference ( or pointer ) if the function isn't taking ownership. So the rule of thumb should be
- As arguments to a function, pass the underlying object by reference or const reference
- As arguments to a functor or function pointer supplied to the standard algorithms,
- pass unique_ptr by reference or const reference
- If you are transferring ownership, then transfer by value
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 24th, 2019, 7:02 pm

Consider using the C++ random library instead of srand()/rand() -> #include <random>
Consider using the C++ chrono library for timer related code.
Looks like C++20 will bring calendar and wall clock time functionality to the chrono library.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Post Reply