Age | Commit message (Collapse) | Author | Files | Lines |
|
This is some rather impressive type safety, albeit safety that has evidently
desensitized people to warnings for rather serious problems.
|
|
PR !270 is an ongoing investigation into outstanding compiler warnings.
One such warning is `warning: format '%s' expects a matching 'char*' argument`
from `script_nullpo_end(nd, STRPRINTF("no such npc: '%s'"_fmt, name));` in
`src/map/script-fun.cpp`. No such warning is emitted from a similar line in
`src/map/atcommand.cpp`: `AString output = STRPRINTF("Jump to %s"_fmt, npc);`.
`script_nullpo_end` is a macro, rather than a function. `error` is passed in
verbatim by the preprocessor. The macro definition of `script_nullpo_end`
includes lines such as the following:
```cpp
PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
```
`#<variable>` instructs the preprocessor to transform `<variable>` into a
literal string. In this case, the string literal:
```cpp
"STRPRINTF(\"no such npc: '%s'\"_fmt, name)"
```
So the entire line partly resolves to something like:
```cpp
printf("script:%s: STRPRINTF(\"no such npc: '%s'\"_fmt, name) @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
```
Once the compiler sees this, it throws out the warning seen here, because
printf is seeing three placeholders in the formatter, but is only given two
value arguments.
Checking with `gcc -E`, the full expansion is as follows:
```cpp
if (nullpo_chk("script-fun.cpp", 4885, __PRETTY_FUNCTION__, nd)) { if (st->oid) { dumb_ptr<npc_data> nullpo_nd = map_id_is_npc(st->oid); if (nullpo_nd && nullpo_nd->name) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " @ %s\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name), nullpo_nd->name); }); } else if (nullpo_nd) { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (unnamed npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } } else { ({ struct format_impl { constexpr static FormatString print_format() __asm__("_print_format") { return "script:%s: " "STRPRINTF(\"no such npc: '%s'\"_fmt, npc)" " (no npc)\n"_fmt; } }; cxxstdio::PrintFormatter<format_impl>::print((
# 4885 "script-fun.cpp" 3 4
stdout
# 4885 "script-fun.cpp"
), (builtin_functions[st->stack->stack_datav[st->start].get_if<ScriptDataFuncRef>()->numi].name)); }); } st->state = ScriptEndState::END; return; };
```
Note the string literal: `"STRPRINTF(\"no such npc: '%s'\"_fmt, npc)"`.
`script_nullpo_end` currently can't take as its argument for `error` any
expression which, when stringified, contains something which `printf` will
interpret as its formatter expecting another argument.
This appears to have been broken since it was first introduced in
https://git.themanaworld.org/legacy/tmwa/-/commit/594eafd4f231a897cbefd
since the first revision implicitly had these requirements, and also
introduced usage which didn't meet them.
This rewrites each PRINTF line in `script_nullpo_end` from the template
```cpp
PRINTF("script:%s: " #error " @ %s\n"_fmt, BUILTIN_NAME(), nullpo_nd->name);
```
to something like
```cpp
PRINTF("script:%s: %s @ %s\n"_fmt, BUILTIN_NAME(), error, nullpo_nd->name);
```
This means that instead of an expression being stringified, error is
interpreted as an expression that resolves (or decays) to a string.
This seems consistent with all current usage of `script_nullpo_end`.
|
|
|
|
|
|
Squash with: remove empty lines
|
|
|
|
C++11 was a really long time ago, huh?
|
|
|
|
|
|
In serverdata@c3b7fe59a, `magic-knuckles` made use of temporary copies of the
spell NPC via `puppet`/`destroy` to provide additional functionality. This
implementation was correct with respect to tmwa behaviour as documented.
However, `puppet` and `destroy` doesn't quite return the server to a clean
prior state. In `builtin_puppet`, events with the fresh new copy of NPC are
registered into the global `ev_db` map to track named events, such as
`OnDischarge`, which might be triggered later. However, these registrations
are not reversed with `destroy`, leaving `ev_db` to retain references to now
invalid and destroyed NPCs.
serverdata@c3b7fe59a revealed this oversight through an intersection of rare
conditions:
- An NPC that is routinely cloned and destroyed
- Having a variety of events
- Including one that can be triggered during a search for the event over ALL
NPCs when `#discharge` is cast.
To reproduce, compile with `-fsanitize=address -fsanitize=undefined` to more
reliably catch use of invalid memory, cast `magic-knuckles`, log out the
character which cast `magic-knuckles` to destroy the NPC, log back in, then
cast `#discharge`. The server will then crash.
Special thanks to SystemError for testing out this theory. Currently lacking
a test environment of my own, this would not have been possible without his
patience and diligence.
|
|
Blame Ledmitz (:
|
|
|
|
Preserve @exprate as a shortcut for both & because scripts in serverdata call it
|
|
|
|
See "Item Mode" commit from Apr 3 2023
|
|
pt item when base stat is 1.
What is funnier is that it sent updates for all other 5 (unchanged
stats). Example, amethyst ring +1 dex:
Sending update for stat 0: saved: 0+1, new: 0+0 (str?)
Sending update for stat 1: saved: 0+1, new: 0+0 (agi?)
Sending update for stat 2: saved: 0+1, new: 0+0 (vit?)
Sending update for stat 3: saved: 0+1, new: 0+0 (int?)
Sending update for stat 5: saved: 0+1, new: 0+0 (luk?)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Item mode
See merge request legacy/tmwa!246
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Equipment stats
See merge request legacy/tmwa!238
|
|
|
|
|
|
|
|
|
|
|
|
Mobinfo
See merge request legacy/tmwa!237
|
|
* includes @mobinfo ingame command aswell as mobinfo functions for scripts
* enhanced summon script command to take a name to support spawn names
* moved @summon to where other mob related commands are
* added enchanter and koyntety cooldown symbols
* some translations
* some constants added for drops and mobs
|
|
|
|
- Fix the lighting machine gun bug
- Fix upmarmu workings
- Reduce nerf factor from 11 to 15
|
|
own limits.
No wonder upmarmu is so powerful, it is easy to be if you ignore max_aspeed parameter.
Also, smooth the aspd nerf.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|