Re: [Smonline-devs] Windows compilation fixed, a few small changes

[Josh <axl...@gm...>]

Gregory Najda wrote:
> Hi, I'm interested in extending the SMO protocol so that songs are 
> identified by a hash in addition to title, subtitle, and artist so 
> that the correct song gets played by all players even if it's a 
> commonly-stepped song and they have more than one stepfile of it. I 
> talked with Charles for a bit a couple weeks ago and I learned that 
> the SVN code is an incomplete new version.
We considered this originally, but issues will arise if the song's 
offset is adjusted.  If you can get it to work, great!
>
> I'd be glad to help you guys get the new code finished and stable and 
> be the Windows maintainer. To start, I've fixed compilation on 
> Windows, created a VS 2008 solution and project file, and made a 
> couple minor changes. I've placed the patch at 
> http://www.cs.stevens.edu/~gnajda/SMO/windows_and_cleanup_249.patch 
> <http://www.cs.stevens.edu/%7Egnajda/SMO/windows_and_cleanup_249.patch> 
> (I'm not sure if attachments get through the mailing list).
>
It will take me a while to comb through the changes.  I would me more 
than happy to add you to the project.
> A summary of the changes:
> - Created VS 2008 solution and project files.
> - Fixed Windows compile, eliminated most warnings.
> - Added copy assignment to DataContainer
>
> - DataContainer: changed
> //m_data = new char[m_numElements];
> to
> m_data = new T[m_numElements]; // uhhhh...this is correct and not the 
> above, right?
>
Yes.

> - Rewrote much of ezSocketsPacket, using a vector instead of an array 
> pointer. Compiler-supplied copying behavior was not correct with an 
> array pointer. I could have either written a copy constructor and copy 
> assignment or made it so the compiler-generated copy functions are 
> correct. The less manual memory management, the better, so I did the 
> latter. Some functions return packets. When you return an object, what 
> normally happens is that the object gets copied using the copy 
> constructor, the copy is the return value, and the original gets 
> destructed. Some compilers optimize away the copy. MSVC does this, but 
> only with optimizations on. With optimizations off (in the Debug 
> configuration), it does not do this optimzation.
I'm a little cautious when making changes to ezSocketsPacket simply 
because it needs to be compatible with previous versions.  You may want 
to talk to Charles about this, as he wrote most of it.
>
> - Made a bunch of things that have destructors but that don't define 
> copy constructor/copy assignment inherit from boost::noncopyable - to 
> prevent copying of objects that shouldn't be copied.
The compiler will make a useable copy constructor, if you don't need a 
deep memory copy.
>
> - Misc. code cleanup.
>
> Some comments:
>
> - Why create your own smart pointer and thread classes instead of 
> using the ones in boost? "AutoPtr" in the name is misleading for a 
> smart pointer that's a reference-counting pointer and not one that 
> deletes the object as soon as the pointer goes out of scope.
AutoPtr deletes the pointer when the reference count reaches 0. Please 
look at AutoPtr::DecrementReference.
> - Prefer vector and string to pointers. The speed cost is not as great 
> as you think and it eliminates a class of bugs.
Note that an EzSockets class can not be placed in a vector.  Copying the 
class will break the socket connection.
> - If a class has a destructor, it should also have both a copy 
> constructor and copy assignment or be uncopyable (since we're already 
> using boost, inherit from boost::noncopyable). Objects returned by 
> functions by value (not by pointer) must have a copy constructor.
> - Don't use variable-length arrays - that's a C99 feature and is not 
> standard C++ (and doesn't compile with MSVC). Use vectors or strings 
> instead.
I have since discovered that variable length arrays are not standard C++.
In the past we have discovered some serious issues with std::vector in 
Visual Studio 2005 and up.  I forget the exact circumstances but it was 
something along the lines of, when storing pointers in a vector, if a 
pointer is deleted, Visual Studio would somehow remove that specific 
index from the vector.  That's why we have been steering a way from vectors.
>
> -#if defined(WIN32)
> typedef unsigned int uint16_t; - wtf? unsigned int is 32 bits!!!
>
WTF indeed.
> So, what remains to be done? And if there's a concensus on how to 
> implement the protocol extension I mentioned, could it be implemented 
> on smonline.us <http://smonline.us>? I am very interested in getting 
> the extension implemented because I host weekly Stepmania meetups for 
> members of a forum and the problem of only title, subtitle, and artist 
> being used to identify stepfiles keeps us from playing some of our 
> favorites. I've already made the necessary changes to the Stepmania 
> client, although that might have to change if a stepfile with 
> added/removed charts is to be considered the "same" stepfile. 
> (http://www.stepmania.com/forums/showthread.php?t=19844)
I would say tack the extension on as a different packet type and make it 
optional, so that it is backwards compatible.
>
> A bit about myself: My name is Greg Najda. I recently graduated from 
> Stevens Institute of Technology as a computer science major. I'm 
> currently unemployed, so I have plenty of time to work on this. :)
> ------------------------------------------------------------------------
>
> ------------------------------------------------------------------------------
> Enter the BlackBerry Developer Challenge  
> This is your chance to win up to $100,000 in prizes! For a limited time, 
> vendors submitting new applications to BlackBerry App World(TM) will have
> the opportunity to enter the BlackBerry Developer Challenge. See full prize  
> details at: http://p.sf.net/sfu/Challenge
> ------------------------------------------------------------------------
>
> _______________________________________________
> Smonline-devs mailing list
> Smo...@li...
> https://lists.sourceforge.net/lists/listinfo/smonline-devs
>