The pattern compare in Blt16BPPBufferPixelateRectWithColor gives neither
operand a size: a memory reference with no register to take the width from,
against an untyped zero. MSVC picks a byte, which is right -- Pattern is
UINT8[8][8], and every other access to it in the block goes through al --
but the source never says so, and clang-cl refuses to guess.
Naming the size changes nothing. cl emits 80 3C 1E 00 before and after.
DrawTextToScreen takes STR16, a mutable wchar_t*, for a string it only
measures and prints. Quest Debug System.cpp picks its text with a conditional:
DrawTextToScreen( gubFact[ usLoop1 ] ? L"True" : L"False", ... );
which has type const wchar_t*. clang-cl's MSVC compatibility lets a bare
string literal decay to wchar_t*, but not a const wchar_t* expression, so it
reports
error: no matching function for call to 'DrawTextToScreen'
note: candidate function not viable: 1st argument ('const wchar_t *')
would lose const qualifier
The parameter is const-correct in every function DrawTextToScreen hands the
string on to, so the change follows the string down the call chain rather
than stopping at the top:
DrawTextToScreen, ShadowText Utils/WordWrap.h
WFStringPixLength Utils/Font Control.h
mprintf, StringPixLength, sgp/Font.h
VarFindFontCenterCoordinates,
VarFindFontRightCoordinates
WinFontStringPixLength sgp/WinFont.h
gprintfdirty TileEngine/Render Dirty.h
The varargs members of that list copy the format string into a local buffer
before doing anything with it, and the measuring ones walk it and return a
width; none of the nine assigns through the parameter. Where StringPixLength
casts the string to UINT16* to walk it, the cast and the local now carry the
const rather than dropping it.
Callers are unaffected: every one already passes something that converts to
const CHAR16*. Both configurations build with no new diagnostics, and the
parse sweep drops from 9 error sites in 7 files to 5 in 3.
To check the claim that nothing writes through these parameters, read the
nine bodies; each is short.
FileWrite took its source as PTR, a mutable void*, though it only reads it:
the buffer goes straight to vfs::tWritableFile::write, which takes a const
vfs::Byte*. STIConvert.cpp pads a file a byte at a time with
FileWrite(hFile, "", 1, NULL);
and a string literal is a const char[1], which PTR cannot hold.
Declaring the parameter const void* matches what the function does and what
the layer below it already asks for. The cast inside becomes const as well.
None of the 431 calls change: every pointer, const or not, converts to
const void*.
JA2EncryptedFileWrite and NewJA2EncryptedFileWrite have the same shape but
transform the buffer on the way out, so they keep PTR.
Verification:
ninja -C build parse # STIConvert.cpp clean
ninja -C build -k 0 # Release, four applications, green
ninja -C build-debug -k 0 # Debug, four applications, green
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AutoCArray takes its allocator and deallocator as template parameters:
template<typename T, typename void*(*Alloc)(size_t) = malloc,
typename void(*Dealloc)(void*) = free>
Alloc and Dealloc are function pointers, so they are values, not types, and
typename has no business in front of them. MSVC ignores the keyword there;
clang stops at it and cannot parse the parameter list, which fails every
translation unit that reaches this header.
Removing the two stray keywords leaves the parameters exactly as they were
meant to be read, with the same defaults of malloc and free. Nothing in the
tree instantiates AutoCArray, so nothing depends on the spelling either way.
Verification:
grep -rn 'AutoCArray' --include=*.cpp --include=*.h . # only this header
ninja -C build parse # sgp_auto_memory.h clean
ninja -C build -k 0 # Release, four applications, green
ninja -C build-debug -k 0 # Debug, four applications, green
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each of these takes the address of an object that has no name:
bool b = client->Startup(1,30,&SocketDescriptor(), 1);
OutputToStream(msg, &(SGP_LOG(s_log.id)));
MSVC allows it as an extension. The temporary lives to the end of the full
expression, which is past the call, so all three happen to work; clang rejects
the address-of outright.
Naming the object changes nothing about when it is built or what is passed,
only that the pointer now refers to something that outlives the statement.
MemMan.cpp already does this with the same logger call:
sgp::Logger::LogInstance memLeak = SGP_LOG(log_id);
Verification:
ninja -C build parse # address-of-temporary class gone: 24 -> 21 sites
ninja -C build -k 0 # Release, four applications, green
ninja -C build-debug -k 0 # Debug, four applications, green
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DEBUG.H once wrapped its declarations in extern "C", and DEBUG.cpp wrapped the
matching definitions to agree. At some point the header's pair was commented
out, both the opening brace and the closing one:
/*
#ifdef __cplusplus
extern "C" {
#endif
*/
That had to happen, because the block would otherwise have swallowed
<stdexcept>, <list> and namespace sgp further down the header. The wrapper in
DEBUG.cpp was left behind, so seven variables are declared with C++ linkage in
the header and defined with C linkage in the source. That is ill-formed. MSVC
takes the linkage of the first declaration it sees, which is the header's, so
the mangled names still agree and the tree still links; clang refuses.
The header is the only place these seven are declared, so removing the
wrapper leaves everything with the C++ linkage it already had. Nothing here
needs to be callable from C.
A second such wrapper remains further down the file, inside #ifdef SGP_DEBUG.
Nothing in the build defines SGP_DEBUG and nothing it contains is declared in
a header, so it is neither compiled nor mismatched, and it is left alone.
Verification:
ninja -C build parse # language-linkage class gone: 60 -> 55 sites
ninja -C build -k 0 # Release, four applications, green
ninja -C build-debug -k 0 # Debug, four applications, green
The seven diagnostics sit on five lines, three of them sharing a single
declaration, and the parse target counts one site per line.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A braced initialiser does not allow a narrowing conversion unless the value
is a constant expression the compiler can prove fits. Every site here holds a
runtime value: WORLD_COLS is the global guiWorldCols, the laptop screen
coordinates are built from iScreenWidthOffset, the sector limits come from the
externalised options. MSVC accepts all of it silently; clang rejects it, and
it is ill-formed.
Nothing about the generated code changes. MSVC was already performing these
conversions; the casts only say so out loud, at the 73 places where it was
happening implicitly.
The invariant: each site is wrapped in a static_cast to the element type the
initialiser already had, and nothing else is touched. No type is widened, no
expression is reassociated, no value is clamped or checked. static_cast rather
than a C-style cast so that a later reader can grep for the narrowings, and so
that none of these can quietly become a reinterpret_cast if a type changes.
Verification:
# every changed line differs only by inserted casts and their parentheses
git show HEAD -U0 | grep '^[+-][^+-]' |
sed -E 's/static_cast<[A-Za-z0-9_]+>//g; s/[()]//g; s/^[+-]//' |
sort | uniq -c | awk '$1 % 2' # no output
git show HEAD -U0 | grep -c '^+[^+]' # 38 lines, 73 casts
ninja -C build parse # narrowing class gone: 129 -> 91 sites
ninja -C build -k 0 # Release, four applications, green
ninja -C build-debug -k 0 # Debug, four applications, green
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drops the last piece of the proprietary Smacker SDK: SMACKW32.LIB, its
header sgp/SMACK.H, and with them the SMACKW32.DLL the game needed at
runtime. Bink is untouched and still needs binkw32.
The two libraries divide the work differently, so Cinematics.cpp changes
more than the call names. SMACKW32.DLL played a file: it kept the playback
clock (SmackWait), blitted converted 16bpp pixels into a surface we handed
it (SmackToBuffer) and pushed the audio to DirectSound itself. libsmacker
only decodes, so this module now:
- keeps the clock. Frames are advanced against GetTickCount() from the
moment the flic starts, skipping frames if the game fell behind, which
is what SmackWait/SmackDoFrame did internally.
- blits itself. Frames come out 8bpp palettised, so the palette is
converted with Get16BPPColor() and the frame is written into the frame
buffer, clipped to the screen. The Y-scaling flags are not handled;
none of the videos shipping with the game set them.
- plays the audio itself, through the SoundPlayFromBuffer() added earlier.
libsmacker hands out audio a frame at a time, so the whole track is
decoded up front, with the video track disabled to keep that pass cheap,
and given a WAV header so the sound module can take it.
libsmacker reads from memory, which lets the file be read straight out of
the VFS. That removes the dance where the .smk was first copied to a real
file under Temp/ because SmackOpen() could only open a path.
SmkInitialize() loses its window and screen size arguments, which only
existed for SmackBufferOpen(). SmkOpenFlic(), SmkSetBlitPosition() and
SmkGetFreeFlic() are no longer declared in the header; nothing outside the
module called them. SMKFLIC is now opaque, Intro.cpp only holds pointers.
The unused SMACK.H include in Cinematics Bink.cpp goes away as well.
Tested by running the Sirtech splash screen (SPLASHSCREEN.SMK, 640x480,
15 fps, one 44.1kHz 16 bit stereo track) under Wine: picture, palette,
placement and playback speed are right.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
refactor
Smacker video carries its audio as PCM that the video decoder produces
itself, so there is no file for the sound module to open. Add
SoundPlayFromBuffer(), which streams from a caller-owned memory buffer.
It deliberately does not go through the sample cache: a full length intro
video decodes to close to 20 MB of PCM, which does not fit in the cache's
memory budget and would evict everything else on the way to failing. The
channel bookkeeping already has a notion of a stream with no cache slot
behind it (uiSample == -1), which is what streamed files use, so the buffer
simply stays the caller's property for as long as the sound plays.
To share the parameter handling, everything SoundStartStream() did after
opening the stream moves unchanged into SoundStartOpenedStream().
No caller yet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
to avoid having to ship a whole ja2.ini for languages that
currently don't
command-line /language:german still works, but INI file is
prioritized
format:
```
[Language]
LANGUAGE = ITALIAN
```
squashed into a single commit because doing it in steps that all
compiled involved a lot of boilerplate that is pointless to commit to
the repository. GH still has the PR for reference.
g_lang, MAX_MESSAGES_ON_MAP_BOTTOM, and GetLanguagePrefix() were
compile-time constants selected by the ENGLISH/GERMAN/... build
define. They're now runtime, defaulting to the same per-exe value the
define used to pick, and overridable at startup from [Ja2 Settings]
LANGUAGE in Ja2.ini.
-
XMLTacticalMessages is filled at runtime from NewTacticalMessages.xml,
never from compiled-in per-language data: all 8 per-language definitions
were the identical all-zero { L"" }. Delete them and define one shared
buffer in Utils/XML_Language.cpp (the loader) instead of pointer-rebinding
it like the static tables — this drops 9 dead 800KB zero-buffers (8
namespaced copies in LanguageStrings.cpp plus the standalone one) and
leaves no bind-ordering hazard for the XML load path.
-
ExportStrings.cpp privately recompiled one language's full text table
by #including the raw _<LANG>Text.cpp inside namespace Loc, keyed off
the exe-level ENGLISH/GERMAN/... compile macro (whichever the build
happened to select). That's redundant with the pointer globals every
other subsystem already uses (Text.h / LanguageStrings.cpp).
Drop the private copy; the unqualified table names in Loc::ExportStrings
now resolve to the global pointer externs, which BindLanguageStrings has
already rebound to the runtime g_lang by the time this runs (EXPORT_STRINGS
ini flag, checked after GetRuntimeSettings in sgp.cpp). gs_Lang (used only
by Loc::Translate for the Polish/Russian byte remap on raw .edt exports) is
now derived from g_lang via ToLocLanguage, so the export always matches
whatever language is actually active instead of a compile-time pick.
-
Editor/popupmenu.cpp and Strategic/Scheduling.cpp kept their own
call-site extern of gszScheduleActions as CHAR16[NUM_SCHEDULE_ACTIONS][20]
after the real definition changed into a rebindable pointer
(CHAR16 (*)[20], LanguageStrings.cpp). MSVC decays the outer array
dimension when mangling globals, so both declarations produce the same
symbol (?gszScheduleActions@@3PAY0BE@_WA) and the mismatch linked
silently -- but the array-typed TUs then indexed the 4-byte pointer
slot itself as string data, so the editor schedule popup and the map
schedule message text read garbage.
-
add text.def to be single-source of truth on symbol names and use it
.much simpler, less error prone if adding more strings
-
add pseudo interface for language state. MAX_SAGES_ON_BOTTOM must always
change in lockstep with g_lang. this isn't foolproof but better
---------
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
the Ja2.rc file needs to be in the add_executable target for the icon
to appear, and that one requires WinMain.
so get rid of dummy.cpp and put sgp/sgp.cpp in there
This one is big, but unless I missed something, should be all be
trivial.
scripted-diff with the following, then manually tweaked whatever needed:
```
if [ $# -ne 3 ]; then
echo "Usage: $0 '<pattern>' '<replacement>' '<filename>'"
exit 1
fi
pattern="$1"
replacement="$2"
filename="$3"
if [ ! -f "$filename" ]; then
echo "Error: File $filename does not exist."
exit 1
fi
sed -i '/'"$pattern"'/ {
:loop
$ !{
N
/'"$pattern"'.*\n.*#endif/ {
s/'"$pattern"'/'"$replacement"'/
s/#else/} else {/
s/#endif/}/
P
D
}
/'"$pattern"'/ b loop
}
}' "$filename"
echo "Replacement complete in $filename"
```
h/t to Grok2 for the sed command
.editorconfig already enforces it but it's getting changed piecemeal in
every commit. just get it done with.
the script:
```
find . -type f -not -path "./.git/*" -not -path "./.vs/*" | while read -r file; do
isFile=$(file -0 "$file" | cut -d $'\0' -f2)
case "$isFile" in
(*text*)
echo "$file is text"
if [[ $(tail -c 1 "$file" | wc -l) -ne 1 ]]; then
echo "" >> "$file"
fi
;;
(*)
echo "file isn't text"
;;
esac
done
```
Additional music can be added by placing sound files into Dataa/Music folder following this naming convention
musicmode_runningNumber.fileformat, so for example:
Mainmenu_001.mp3
Mainmenu_002.ogg
Laptop_002.ogg
Tactical_003.wav
*****************************************************************
Different musicmodes are
Mainmenu_ <-- Mainmenu music
Laptop_ <-- Laptop music
Tactical_ <-- Tactical with nothing special going on
Enemy_ <-- Tactical, enemy present
Battle_ <-- Tactical, enemy present and visible
Victory_ <-- Tactical, battle victory
Death_ <-- Tactical, battle defeat
Creepy_ <-- Tactical, creatures present
CreepyBattle_ <-- Tactical, creatures present and visible
Music files can be .mp3, .ogg or .wav formats and 100 songs in each category is supported.
The running numbering starts from 000
*****************************************************************
A few filenames are treated in a special way to facilitate easy replacement of original music.
To replace original music, insert the following named files into this folder
"menumix1" <-- Mainmenu music
"marimbad 2" <-- Laptop music
"nothing A" <-- Tactical with nothing special going on
"nothing B"
"nothing C"
"nothing D"
"tensor A" <-- Tactical, enemy present
"tensor B"
"tensor C"
"triumph"
"death"
"battle A" <-- Tactical, enemy present and visible
"tensor B" <-- This file is also used as battle music
"creepy" <-- Tactical, creatures present
"creature battle" <-- Tactical, creatures present and visible