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

[Gregory N. <gre...@gm...>]

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.

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 (I'm not
sure if attachments get through the mailing list).

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?

- 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.

- 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.

- 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.
- Prefer vector and string to pointers. The speed cost is not as great as
you think and it eliminates a class of bugs.
- 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.

-#if defined(WIN32)
typedef unsigned int uint16_t; - wtf? unsigned int is 32 bits!!!

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?
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)

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. :)