The parameter was STR8, a mutable char*, for a filename the function only
reads: it runs strstr over it and hands it to SoundPlay. environment.cpp picks
between two of them at runtime,
PlayJA2SampleFromFile( Chance( 50 ) ? "Sounds\\WATERSNAKE_ATTACK_01.WAV"
: "Sounds\\WATERSNAKE_ATTACK_02.WAV", ... );
and the conditional is a const char*, which STR8 has no room for. Literals
passed on their own are still accepted under the MSVC rules, which is why only
these two calls ever failed.
The cast at the call to SoundPlay was already written as (STR) and is left as
it is. SoundPlay, SoundPlayStreamed and SoundLoadSample below it all still
take STR, so making them const is a separate piece of work and a much larger
one; until then that cast is where const stops.
No call site changes. The (char*) cast at LuaInitNPCs.cpp:11640 is now
unnecessary but still valid, and is left for whoever next edits that line.
Verification:
ninja -C build parse # both environment.cpp sites gone
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>
STR16 is CHAR16*, so ScreenMsg asked for a mutable format string it has no
intention of mutating: the body only hands it to va_start and vswprintf, and
vswprintf takes it as const.
Calls that pass a literal are accepted anyway under the MSVC rules clang-cl
applies. Calls that pass anything else are not, and DynamicDialogue.cpp picks
the format at runtime:
ScreenMsg( FONT_MCOLOR_LTGREEN, MSG_INTERFACE,
...sOpinionModifier >= 0 ? L"%s: %s +%d" : L"%s: %s %d", ... );
The conditional has type const wchar_t*, and there is nowhere for it to go.
Declaring the parameter const CHAR16* is what the function always meant. Every
existing caller still compiles: a literal and a CHAR16* both convert.
Verification:
ninja -C build parse # DynamicDialogue.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>
STR8 is char*, so const STR8 is char* const: a const pointer to a mutable
string, which is not what any of these sixty parameters wanted. What they
wanted is const CHAR8*, a pointer to a string they will not write to, and
that is what all of them do.
Under the MSVC compatibility rules clang-cl applies, a string literal still
converts to char*, so most calls survive this mistake untouched. An expression
of type const char* does not, and Intro.cpp has thirteen of them:
inireader.ReadString("INTRO_BEGINNING", "INTRO_REBEL_CRDT",
no_defaults ? "" : "INTRO\\Rebel_cr");
The conditional is a const char*, not a literal, so it has nowhere to bind and
the call does not compile. The other two arguments are literals and are
accepted, which is why only the third one was ever reported.
The invariant: in these two files, every parameter spelled const STR8 becomes
const CHAR8*, and nothing else changes. No body assigns to one of them, so
none needed rewriting; STR8 input_buffer in the five-argument ReadString is
genuinely an output and keeps its type. Three commented-out lines still say
const STR8 and were left as they are.
Verification:
grep -n 'const[ \t]*STR8' Utils/INIReader.h Utils/INIReader.cpp
# three hits, all on lines beginning with //
git show HEAD -U0 | grep '^[+-][^+-]' |
sed -E 's/const[ \t]+(STR8|CHAR8\*)/const/g; s/^[+-]//' |
sort | uniq -c | awk '$1 % 2' # no output
ninja -C build parse # ReadString class gone: 45 -> 32 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>
Both of these bound a non-const reference to a temporary, which MSVC accepts
and clang does not, and which leaves the reference dangling once the full
expression ends.
PropertyContainer::initFromXMLFile and writeToXMLFile take their tag map by
non-const reference because the accessors on it insert default tag names as
they are asked for, so it cannot be const and it cannot be a temporary. All
eight call sites passed a freshly built one; they now declare it. Each keeps
its own object, so a map filled in by one call cannot leak into the next, the
same as when each call built its own temporary.
XMLWriter::writeToFile bound std::string& to what stringstream::str() returns
by value. Declared as a value it is initialised straight from the return with
no copy, and const because only c_str() and length() are asked of it.
Verification:
grep -rn 'PropertyContainer::TagMap()' . # nothing
ninja -C build parse # no i18n or XMLWriter sites remain
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>
POPUP_OPTION copies the name it is given -- `this->name = *newName` into a
std::wstring member -- but its constructor took a std::wstring*, so 64 call
sites spelled the argument `&std::wstring( pStr )`. Taking the address of a
temporary is ill-formed, and clang refuses it outright:
error: taking the address of a temporary object of type 'std::wstring'
It happens to work under MSVC because the temporary outlives the call, dying
at the end of the full expression rather than before the copy. Nothing was
corrupt; the code was just spelling "pass me a string" in a way the language
does not allow.
A const reference says what these functions actually want, so the call sites
lose the &, and the two places that allocated a string purely to have an
address to pass -- POPUP_SUB_POPUP_OPTION's default constructor, which carried
a "TODO: possible memmory leak!" saying as much, and a "Dummy generator"
option -- stop leaking one.
Converted: POPUP_OPTION's constructor and setName, POPUP::addOption and
addSubMenuOption, and both POPUP_SUB_POPUP_OPTION constructors. The popupDef
family in popup_definition.* keeps its std::wstring* because it owns what it
is handed and stores the pointer; that is a different design and a different
change. The three calls it makes into the converted API now dereference.
Verification:
grep -rn '&std::wstring' --include=*.cpp --include=*.h . # nothing
ninja -C build parse # no popup_* or SkillMenu sites remain
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
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>
Added detection of the NEWLINE_CHAR character "±" to enable line breaks not only when text exceeds display width, but also upon encountering NEWLINE_CHAR.
Note: This character has been conventionally adopted in the Chinese community as a line break marker within XML item descriptions. The modification affects Chinese text display only.
most likely every windows install has en_US available, even if in other
languages. if not, bring back the insertcommas function from history, no
biggie
* Use the new columns added to Text instead
* Changed most functions to allow choosing a column
* Removed unneeded SecondColumn specific functions to lessen code duplication
* Lots of formatting changes to Popupbox.cpp, indentation was messed up in majority of the functions
eventually we'll switch between CHINESE and other languages during
runtime, so even if language is not chinese, these symbols must still
be there
these symbol names aren't used so this is fine
Requires accompanying GameDir change.
Removed ItemFlag as an xml tag and added bitmask flags as their own individual tags. Makes modding easier when the flags can be modified without math in the xml files
simplified the previous commit, now no more need for a tag
when checking IsItemLegal for Food System, ontop of checking for foodpoints, now also check if is drug
as a result, as previously, all eatable food will be excluded without food system, but now only if it isn't a drug at the same time
and no more tag to check for that in items.xml needed
no food items if the food system is off
whether the item is exclusive is defined by tag in items.xml
replaced previous way of decision, which used Item[usItemIndex].foodtype > 0 as criteria
that also restricted dual use items (i.e. drugs that also have a foodtype, like coffee, etc.)
.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
```
- face protection had a misleading text, stating item should be worn at face
- that's no longer true, so adjusted text to avoid player confusion
- adjusted text to distinguish between both disease protections variations
JuqiandAsdow
BeatAroundTheBuscherandGitHub
FunkyFr3shandmajcosta
kitty624
momoko-handGitHub