Help save net neutrality! Learn more.
Close

#44 Move class Mount into own header to reduce unit_generic LOC

open
nobody
None
5
2012-11-10
2012-11-10
logzero
No

I would like to move MeshAnimation class out of unit_generic too.
Additionally unit_generic could be split into subclasses UNIT_AI, UNIT_CARGO, UNIT_FLIGHTGRP etc to make it a bit more readable.
This changes would be transparent to vegastrike functionality and constrained to unit_generic.

Discussion

  • logzero

    logzero - 2012-11-10

    patch

     
  • Klauss++

    Klauss++ - 2012-11-10

    Why did you remove includes from mount.cpp? Are you absolutely sure they're not need?

    Note that they may still be included indirectly, and that would make it build fine, but cause us trouble down the road Ie: if we were to remove the include from those files, it would cascade in an unpredictable manner.

    Explicitly relying on recursive includes is usually not recommendable for a number of reasons. So... again... are you sure you can remove them? lin_time, for instance, I'm sure it's used (there's a call to realTime)

     
  • logzero

    logzero - 2012-11-10

    I see. Ideally the headers should have a minimum of includes themselves. Now with vegastrike it is hard to see "see the forest for the trees" so I tried to reduce the includes to get an idea of what is going on, should have kept this changes local I think. Feel free to reject the patch.

    Btw I can't comment on the const reference patch anymore, but a default temporary parameter for a const reference is valid c++ afaik. The std::vector constructor has one for example, something like: const T& value= T()

     
  • Klauss++

    Klauss++ - 2012-11-10

    It's standard, but doesn't always work on all compilers. That's why we steer clear of it.

     
  • logzero

    logzero - 2012-11-12

    This one can be closed/rejected too, won't be pushing for it.

     
  • Klauss++

    Klauss++ - 2012-11-13

    I'm not opposed to the move. Only to include removal. I haven't had time to carefully review the fallout either (I'm battling with local changes to those files myself). That doesn't mean I'll reject it.

     

Log in to post a comment.