On Mon, 2024-02-12 at 15:01 -0500, Thomas Frohwein wrote:
> On Mon, Feb 12, 2024 at 07:12:56PM +0100, Martijn van Duren wrote:
> > $ cat stratagus/pkg/DESCR 
> > Stratagus is a free cross-platform real-time strategy gaming engine. It
> > includes support for playing over the internet/LAN, or playing a
> > computer opponent. The engine is configurable and can be used to create
> > games with a wide-range of features specific to your needs. Stratagus is
> > written in C++. 
> > 
> > This is a dependency for war1gus and wargus, a warcraft orcs and humans,
> > and warcraft 2 open source reimplementation respectively (need original
> > assets).
> > 
> > Both the changes to the CMakeLists.txt are probably wrong/not needed,
> > but outside the scope of my knowledge, so if someone would at least
> > look those over it would be appreciated.
> 
> Thanks for sharing this! I looked at it a little; this is a trickier
> project. The main thing I'm bumping into is that you set
> "USE_NINJA=No" which doesn't seem to be intended (either "yes" or
> "samurai"). The result is that none of the flags from
> MODCMAKE_BUILD_TARGET are used and instead it builds with cmake with
> no arguments. When I try using our regular MODCMAKE_BUILD_TARGET (by
> removing the USE_NINJA line), it then somehow doesn't build the
> vendored lua dependencies including the needed .a file before it's
> needed. I would have thought that this line in CMakeLists.txt should
> ensure that:
> 
> add_dependencies(stratagus Lua51B)
> 
> but that doesn't seem to be the case. Maybe someone with more cmake
> knowledge can better spot why this only works with empty
> MODCMAKE_BUILD_TARGET.

This build issue is the reason why I found that USE_NINJA=no fixed it.
But if someone with more cmake knowledge knows how to fix this I'm
definitely not objecting. :-)
> 
> Here a few other nits:
> 
> - remove leading article in COMMENT

Sure

> - drop devel/sdl2 LDEP (is pulled in by sdl2-mixer and sdl2-image)
>   -> same for audio/libogg

If they're direct dependencies tested for by the configure script,
shouldn't they be explicitly noted?

> - setting WRKDIST not be needed as DISTNAME already set correctly

Right. Minor left-over from when I fudged around with the package name.

> - disable doxygen? (low quality docs; don't set ENABLE_DOC)

I haven't really looked at the output of doxygen. But the doxygen
part is needed to install the manpage. This can of course be done
with a workaround, but didn't thought it'd be the effort compared to
this flag.

> - add archivers/innoextract to RDEP? (see patches - this is used by runtime)

I've added these to war{,1}gus, since it's in the code that's in the
.h files (yes... I know...), so it's war{,1}gus binary that calls this
function and not the stratagus binary. But there's something to say to
let stratagus have the dependency since it's its header... I'll change
it if you still feel it's in the wrong place as is.

> - variables USE_OPENMP and USE_BACKTRACE are not used by cmake (see
>   make configure output)
> 
You're right. USE_OPENMP was needed for the head of tree, apparently
it detects it correct in the release. Same for USE_BACKTRACE.
> 
> PS:
> 
> $ make show=MODCMAKE_BUILD_TARGET
> cd /usr/ports/pobj/stratagus-3.3.2/build-amd64 && exec /usr/bin/env -i 
> MODCMAKE_USE_SHARED_LIBS=yes MODCMAKE_PORT_BUILD=yes PORTSDIR="/usr/ports" 
> LIBTOOL="/usr/bin/libtool"  
> PATH='/usr/ports/pobj/stratagus-3.3.2/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11R6/bin'
>  PREFIX='/usr/local'  LOCALBASE='/usr/local' X11BASE='/usr/X11R6'  
> CFLAGS='-O2 -pipe'  TRUEPREFIX='/usr/local' DESTDIR=''  
> HOME='/stratagus-3.3.2_writes_to_HOME' PICFLAG="-fpic"  BINGRP=bin 
> BINOWN=root BINMODE=755 NONBINMODE=644  DIRMODE=755  INSTALL_COPY=-c 
> INSTALL_STRIP=-s  MANGRP=bin MANOWN=root MANMODE=644 
> BSD_INSTALL_PROGRAM="/usr/ports/pobj/stratagus-3.3.2/bin/install -c -s -m 
> 755"  BSD_INSTALL_SCRIPT="/usr/ports/pobj/stratagus-3.3.2/bin/install -c -m 
> 755"  BSD_INSTALL_DATA="/usr/ports/pobj/stratagus-3.3.2/bin/install -c -m 
> 644"  BSD_INSTALL_MAN="/usr/ports/pobj/stratagus-3.3.2/bin/install -c -m 644" 
>  BSD_INSTALL_PROGRAM_DIR="/usr/ports/pobj/stratagus-3.3.2/bin/install -d -m 
> 755"  BSD_INSTALL_SCRIPT_DIR="/usr/ports/pobj/stratagus-3.3.2/bin/install -d 
> -m 755"  BSD_INSTALL_DATA_DIR="/usr/ports/pobj/stratagus-3.3.2/bin/install -d 
> -m 755"  BSD_INSTALL_MAN_DIR="/usr/ports/pobj/stratagus-3.3.2/bin/install -d 
> -m 755"  cmake --build /usr/ports/pobj/stratagus-3.3.2/build-amd64 --verbose 
> -j 1
> 
Not quite sure what you're trying to show here. Sorry.

> > One issue is that sound doesn't play with video cut scenes and is
> > something that boils up from SDL2's Mix_LoadMUS or Mix_LoadMUS_RW in
> > stratagus' src/sound/sound_server.cpp's LoadMusic().
> > 
> > martijn@
> 
> 

Attachment: stratagus.tar.gz
Description: application/compressed-tar

Reply via email to