[Scummvm-devel] [Scummvm-cvs-logs] SF.net SVN: scummvm:[42257] scummvm/trunk/graphics

Filippos Karapetis philipk79 at hotmail.com
Wed Jul 8 19:28:13 CEST 2009


Hi everyone

This commit actually brought into surface a problem with the compilation process under MSVC. To illustrate, here
is the current compilation process under MSVC:

1>------ Build started: Project: kyra, Configuration: Debug Win32 ------
1>   ..\..\engines\kyra\screen.cpp
(ENABLE_KYRA not defined - snipped loads of errors)
1>kyra - 26 error(s), 1 warning(s)
2>------ Build started: Project: scummvm, Configuration: Debug Win32 ------
2>Compiling...
(snip)
2>   ..\..\graphics\sjis.cpp
2>sjis.cpp
(ENABLE_KYRA defined - no errors)
2>Linking...


So the *actual* error for this issue is that ENABLE_KYRA is not defined when compiling the kyra engine
(inside kyra.vcproj) and it's only defined when compiling the overall solution (inside scummvm.vcproj).

The reason why we didn't have problems so far is explained by LordHoto as follows:
<LordHoto> engines typically use:
<LordHoto> #if PLUGIN_ENABLED_DYNAMIC(KYRA) REGISTER_PLUGIN_DYNAMIC(KYRA, PLUGIN_TYPE_ENGINE, KyraMetaEngine);
<LordHoto> #else REGISTER_PLUGIN_STATIC(KYRA, PLUGIN_TYPE_ENGINE, KyraMetaEngine);
<LordHoto> #endif
<LordHoto> so if ENABLE_KYRA isn't specified and the detection.cpp file is compiled it'll assume it's statically linked
<LordHoto> thus that's why there's no need for MSVC to include that define in the engine project files so far

There are two ways to address this issue:
1) Add ENABLE_XXX defines in all engine MSVC project files
i.e. ENABLE_AGI in agi.vcproj, ENABLE_SAGA in saga.vcproj etc (one define per engine in each engine project file).
This means that we'll need to make 2 defines when we want to enable an engine: one in scummvm.vcproj (to 
include the engines) and one in the engine MSVC project files themselves so that they can use engine-specific
common code. This can (and will) get messy and ugly, eventually.
2) Add common includes for all engines, thereby greatly simplifying the engine project files, and having everything
neatly packed in one common file for all MSVC project files. This solves not only this problem, but many others too,
as all compilation options are in one file and not scattered among different ones - this actually makes MSVC
compilation options a bit more similar to the GCC ones (i.e. they're all in one place). This solution has been implemented
by nolange in the following rejected patch:
https://sourceforge.net/tracker/?func=detail&aid=2774908&group_id=37116&atid=418822
The main problem with this solution is that it's only supported in MSVC8 and later (as described in the patch tracker
item), so we'll have to drop MSVC7 and MSVC7.1 - unless we move to some better system than the batch files I've
made right now, i.e. cmake or similar

Personally, I vote for the second solution: it would be better to drop support for older/strange compilers rather than
keeping compile options and defines scattered and duplicated in several files. Also, not a lot of users use MSVC7 and
MSVC7.1 (I only know of 1, Quietusk). 

However, I'd like to discuss this before doing anything...
Should we drop support for older MSVC compilers and address this properly, or modify all MSVC project files to have
an ENABLE_XXX define?

Regards
Filippos

> CC: scummvm-devel at lists.sourceforge.net
> From: max at quendi.de
> Date: Wed, 8 Jul 2009 15:33:51 +0200
> Subject: Re: [Scummvm-devel] [Scummvm-cvs-logs] SF.net SVN: scummvm:[42257]	scummvm/trunk/graphics
> 
> 
> Am 08.07.2009 um 09:13 schrieb thebluegr at users.sourceforge.net:
> 
> > Revision: 42257
> >          http://scummvm.svn.sourceforge.net/scummvm/? 
> > rev=42257&view=rev
> > Author:   thebluegr
> > Date:     2009-07-08 07:13:56 +0000 (Wed, 08 Jul 2009)
> >
> > Log Message:
> > -----------
> > The ENABLE_* flags are only checked for *.cpp files in MSVC, so move  
> > the ENABLE_* checks to sjis.cpp to fix compilation under MSVC
> >
> 
> I don't understand, what does that mean??? Do you mean to say these  
> flags are only #defined in the .cpp files? That would be most weird.
> 
> Or maybe MSVC doesn't #define them globally at all but rather relies  
> on these values being set from config.h -- in that case, the solution  
> would be to add an #include "common/scummsys.h" at the top of the  
> header.
> 
> But moving the check from the header file to the source file seems  
> wrong.
> 
> Cheers,
> Max
> 
> 
> > Modified Paths:
> > --------------
> >    scummvm/trunk/graphics/sjis.cpp
> >    scummvm/trunk/graphics/sjis.h
> >
> > Modified: scummvm/trunk/graphics/sjis.cpp
> > ===================================================================
> > --- scummvm/trunk/graphics/sjis.cpp	2009-07-08 07:11:43 UTC (rev  
> > 42256)
> > +++ scummvm/trunk/graphics/sjis.cpp	2009-07-08 07:13:56 UTC (rev  
> > 42257)
> > @@ -23,11 +23,16 @@
> >  */
> >
> > #include "graphics/sjis.h"
> > +#include "common/debug.h"
> >
> > -#ifdef GRAPHICS_SJIS_H
> > +// The code in this file is currently only used in KYRA and SCI.
> > +// So if neither of those is enabled, we will skip compiling it.
> > +// If you plan to use this code in another engine, you will have
> > +// to add the proper define check here.
> > +// Also please add the define check at the comment after the
> > +// matching #endif further down this file.
> > +#if defined(ENABLE_KYRA) || defined(ENABLE_SCI)
> >
> > -#include "common/debug.h"
> > -
> > namespace Graphics {
> >
> > bool FontTowns::loadFromStream(Common::ReadStream &stream) {
> > @@ -193,5 +198,5 @@
> >
> > } // end of namespace Graphics
> >
> > -#endif // defined(GRAPHICS_SJIS_H)
> > +#endif // defined(ENABLE_KYRA) || defined(ENABLE_SCI)
> >
> >
> > Modified: scummvm/trunk/graphics/sjis.h
> > ===================================================================
> > --- scummvm/trunk/graphics/sjis.h	2009-07-08 07:11:43 UTC (rev 42256)
> > +++ scummvm/trunk/graphics/sjis.h	2009-07-08 07:13:56 UTC (rev 42257)
> > @@ -22,14 +22,6 @@
> >  * $Id$
> >  */
> >
> > -// The code in this file is currently only used in KYRA and SCI.
> > -// So if neither of those is enabled, we will skip compiling it.
> > -// If you plan to use this code in another engine, you will have
> > -// to add the proper define check here.
> > -// Also please add the define check at the comment after the
> > -// matching #endif further down this file.
> > -#if defined(ENABLE_KYRA) || defined(ENABLE_SCI)
> > -
> > #ifndef GRAPHICS_SJIS_H
> > #define GRAPHICS_SJIS_H
> >
> > @@ -131,5 +123,3 @@
> >
> > #endif
> >
> > -#endif // defined(ENABLE_KYRA) || defined(ENABLE_SCI)
> > -
> >
> >
> > This was sent by the SourceForge.net collaborative development  
> > platform, the world's largest Open Source development site.
> >
> > ------------------------------------------------------------------------------
> > 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
> > _______________________________________________
> > Scummvm-cvs-logs mailing list
> > Scummvm-cvs-logs at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/scummvm-cvs-logs
> >
> 
> 
> ------------------------------------------------------------------------------
> 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
> _______________________________________________
> Scummvm-devel mailing list
> Scummvm-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/scummvm-devel

_________________________________________________________________
Hotmail® has ever-growing storage! Don’t worry about storage limits. 
http://windowslive.com/Tutorial/Hotmail/Storage?ocid=TXT_TAGLM_WL_HM_Tutorial_Storage_062009
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20090708/9473c9f4/attachment.html>


More information about the Scummvm-devel mailing list