Cracktro :)

The Partridge Family were neither partridges nor a family. Discuss.
albinopapa
Posts: 4171
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 1:50 am

Long story short, templates provide:
  • Code reuse without polymorphism
  • Compile time checking of types
  • Possibly compile time error checking
  • Static dispatch instead of dynamic dispatch
  • More efficient code generation since templates MUST be defined in headers if declared in headers.
  • and probably more I can't think of right now
It has a couple drawbacks though, code bloat and of course ambiguity unless you use something like enable_if to constrain your classes and functions. They can be harder to debug as well since the error message will usually point to something in the template code instead of the point in regular code that caused the template to be ill-formed.
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: 4171
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 2:45 am

Just looked through your XMLPlayer code and came across the StyleGuide.h file and see a lot of what I suggested is in there and then some. Also, for that project I don't see a need for templates anywhere, so I can definitely see why you might think templates would be a waste of time.

Whoops, don't know if you wrote the VirtualFile stuff, but I thought this was kind of funny.

Code: Select all

        assert( quantity <= sizeof( unsigned ) << 3 ); // read max 32 bits
The comment says "read max 32 bits, however, the standard doesn't guarantee that an unsigned int is 32 bits. It only says that an unsigned int is at least 16 bits. It would probably be better to just say: assert( quantity <= 32 ); for clarity and portability.

If you are opposed to writing out 'unsigned int' perhaps use the standard int type:
  • int8_t/uint8_t guaranteed to be 8 bits
  • int16_t/uint16_t guaranteed to be 16 bits
  • int32_t/uint32_t guaranteed to be 32 bits
  • int64_t/uint64_t guaranteed to be 64 bits
Expressive, brief and portable.

I'll wait for your refactoring, because that VirtualFile class...dang. :p

EDIT: Turns out char, signed char and unsigned char are all different so for strings, stick with 'char' and if you plan on doing math on an 8 bit variable, then use int8_t or uint8_t since 'char' can be either signed or unsigned depending on the compiler. Just learned that today.
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 » November 8th, 2019, 9:57 am

Hi Albinopapa!
The idea behind the assert() in virtualFile.h is that the function should not read more bits than that an integer of type "unsigned" can hold, whether "unsigned" is a 16, 32 or 64 bit integer. In that sense the comment is wrong (or simplified) more than the function. It is a bit worrysome you dislike the virtualfile() class so much, since it's kind of the most recent addition to the XM player project :D I did write it in a hurry, so it is quite basic qua error handling and such, to say the least. The bitRead functions were made to be used in itsex.h / .cpp but I finally chickened out and left that code as it is (it was c code not written by myself, I just wrapped it in a class), since I found the sample decompression routine a bit complex, and not interesting enough to figure out 100% how it works ;)
I do use chars the way you describe it (mostly). I refrained from using int8_t, int16_t etc because I thought these were Microsoft - only. I had no idea they were part of the standard, and thought the opposite was true. Especially since other programmers that write similar software use there own typedefs and go like:

Code: Select all

typedef unsigned char BYTE;
typedef unsigned short WORD;
typedef signed short SHORTINT;
typedef int LONGINT;
typedef unsigned DWORD;
I found this to be a bit heavy-handed and wanted to use standard C(++) as much as possible. No idea how successful I was in that sense, never dared to compile it using clang or something like that ;)

About the template meta programming: thanks for clearing that up, I didn't think of the early vs. late binding aspect. I'll keep it in mind, even though my mind is getting a bit full now ;)

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

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 10:53 am

I can't say I have a problem with the VirtualFile class itself and if is just copy/paste code from another library then I can't really fault you for the bit manipulation nightmare inside.

I do have a piece of advice though concerning the function template

Code: Select all

    template<class PTR> MemoryBlock<PTR> getPointer( uint32_t nElements )
    {
        uint32_t byteSize = nElements * sizeof( *PTR );
        /* ... */
    }
What happens if PTR is not a pointer? C++ has static_assert for compile time assertions and here's a good place for it.

Code: Select all

    template<class PTR> MemoryBlock<PTR> getPointer( uint32_t nElements )
    {
        static_assert( std::is_pointer_v<PTR>, "VirtualFile::getPointer() template parameter must be a pointer type." );
        uint32_t byteSize = nElements * sizeof( *PTR );
        /* ... */
    }
In regards to the typedefs I have two things to say.
1) the typedefs that I posted have been in the C standard for a while, they aren't specific to C++, but you can prefix them with std:: if you wanted, but isn't necessary. Microsoft typedefs are almost exclusively CAPITALIZED like UINT, DWORD and the like.
2) Please for the love of God, don't use typedef. I pray for the annihilation of that syntax some day. C++11 gave us the 'using' keyword to create aliases. It is more flexible since you can use it with templates and the interface is much cleaner and clearer. C reads right to left
DEFINING A TYPE ALIAS
typedef unsigned int DWORD;

C++11 onward is trying to move to left to right reading

using DWORD = unsigned int;

DEFINING A FUNCTION POINTER TYPE
void Func() {}

typedef void( *fn )( );

fn mfunc = &Func;

using Fn = void( *)( );

Fn mFunc = &Func;

Aside from that, to me typedef is misleading as you aren't defining a type, you are defining an alias. Using isn't much better in that regard, but it is logical to say "I'm using DWORD instead of unsigned int".

I believe when it was first introduced in C, it WAS a way of defining a new type because C's structs and enums are declarations unless you prepend them with typedef.

Code: Select all

typedef struct MyStruct_{ /*members*/ }MyStruct;  
or

Code: Select all

struct MyStruct_{ /*members*/ };
typedef Mystruct_ MyStruct;
Because this:

Code: Select all

struct MyStruct{ /*members*/ }MyStruct;
If I recall correctly creates a variable named MyStruct of type MyStruct_ without the typedef decoration. It's been awhile since I programmed in C, so I may have that wrong. I know that's the behavior in C++ ( creating a variable instead of defining an alias).

As you can see, I'm pretty adamant about switching people over to using 'using' instead of typedef for aliasing. It's a small thing to some and old habits type hard I suppose ( ahem, chili, ahem WHA? ).

As to the template comment, what do you mean by "early vs late binding"? That wasn't in the list of pros LOL.
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 » November 8th, 2019, 2:29 pm

Hi Albinopapa :)
My God you reply fast, don't you sleep ;) It's like 8 o' clock in the morning in Oklahoma, you're hardcore man!

Anyway, about the MemoryBlock template class: IIRC I was trying to make my own version of something like std::unique_ptr. The code was never used, I left it there so I could maybe finish it later on. Hence the comment next to the class declaration.

I don't really know how the "using" keyword works, I did not know you could use it like it. But duly noted, typedefs == bad, got it :) I agree with what you said about it for sure. I used it thrice in the whole project I believe. I will not be sad to see them go.

"early vs late binding" is what I understood when you wrote "Static dispatch instead of dynamic dispatch" (I had to google that, I didn't know what it meant). Maybe those terms can't be used interchangeably?

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

Re: Cracktro :)

Post by Byteraver » November 8th, 2019, 5:24 pm

Oh, and just to be in the clear: I assume full responsibility for the "bit manipulation nightmare inside". Euh, Blame Intel's endianness for that. The code that I did NOT write myself is in itsex.c.

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

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 6:00 pm

Actually, I finished the post at nearly 5:00 am here in Oklahoma, then I went straight to bed. I'm a bit of a night owl as it were, but lately I've been trying to go to bed earlier, just failed at it last night.

The getPointer() function is from VirtualFile actually, not MemoryBlock. I did try looking for uses in other parts of the code ( well, Visual Studio did the searching ), but didn't see any. Maybe not as a unique_ptr, but as a random access view into the file, now that might be helpful actually. I hate having to use iostream::seekg() to skip around in the file. Just be cautious when you implement something like that. For instance, your "unique_ptr" like class as a wrapper for another objects memory isn't a good idea. For one, you have to make sure the object you are borrowing from lives longer than the object doing the borrowing. For another, since you're borrowing, you must remember not to "free" the memory as there was no allocations done.

Other than what I listed for 'using' vs 'typedef', the 'using' keyword version works well with templates like creating a "alias template?" or "template alias?" not sure how to word that.

For instance:

Code: Select all

template<typename T> MyPointer = std::unique_ptr<T>;

//usage
MyPointer<int> my_unique_int_pointer = std::make_unique<int>( 42 );
That is something 'typedef' cannot do as far as I'm aware.

After searching on google a bit, it seems binding and dispatch in this context (early binding == static dispatch and late binding == dynamic dispatch ) can be used interchangeably.
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: 4171
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 6:07 pm

Byteraver wrote:
November 8th, 2019, 5:24 pm
Oh, and just to be in the clear: I assume full responsibility for the "bit manipulation nightmare inside". Euh, Blame Intel's endianness for that. The code that I did NOT write myself is in itsex.c.
LOL, "open mouth insert foot" on my part.

So you are saying that the readBit function reverses the order of the bits in the 'lastBitContainer' variable then loads more bits when it gets to the end of the variable's bits?
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: 4171
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » November 8th, 2019, 6:59 pm

Something I noticed you doing a lot is calling c_str() for std::cout and I'm not entirely sure why other than perhaps it's something left over from using one of the C stream functions like printf. Std::cout can handle std::string just fine.
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 » November 8th, 2019, 7:03 pm

Hi again :) When I was home alone in the summer holidays and I didn't see a living soul for a few weeks (our house was in the mountains, South of France, on the border with Spain), my routine also shifted to working till 5 am, sleeping till 3 pm. I wrote my asm mixing routine back then :mrgreen: . I don't do that anymore as it depresses me to live at night. The great thing about it though is that you are 100% sure nobody will bother you. No lunch / dinner time, and everybody is asleep. Very zen times :) Good to stay focused.

I find your description of the bit manipulation thingy quite accurate, no worries. About your question: I don't remember :lol: and I kind of don't want to get back to it, I don't have a fond memory of it :D

The "safe"pointer function is an absolute disgusting hack, no doubt about it. I have to redo that properly. It will be less efficient afterwards though. You see, I have to pass the sample data to the constructor of the Sample class that does copy the data. If I copy it in the getsafepointer fn, then the sample data gets copied twice over... Or more accurately, it gets copied, then converted (all unsigned data is converted to signed for the internal mixer, all 8 bit data is converted to 16 bit data).

So the usage of the getsafepointer goes like: get a pointer to a memory block inside the virtual file, set the pointer in the smpHdr struct, initialize the Sample class using its constuctor with this smpHdr. The data gets converted in Sample.cpp.

Maybe I'll redo it using a const vector reference or smthg. The virtualFile var's are used in the MOD, S3M, IT and XM module file loaders (all in a separate cpp file).

About using .c_str() in std::cout: will find that and fix it, there is just so much to do.

Post Reply