Remove code smells from stonesense, pt. 1
#125
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this mostly replaces `#define`s with `constexpr`s, in line with modern coding standards. use of C-style `typedef struct` eradicated in a couple places. bespoke `null` replaced with standard `nullptr` a handful of methods that could be made `const` were made `const`
`ConnectionState` and `TiletypeMatcher` had no content, no point in compiling them or keeping them around
convert to header-only class rewrite constructors to modern standards
move constructors to header replace explicit destructor with default remove unneeded includes remove `using namespace std` push local function into anonymous namespace `int` -> `size_t` fixes
remove `using namespace std` use ranged `for` use `size_t` instead of `int` use `%` instead of repeated subtraction
remove unneeded exports push nonexported functions into anonymous namespace move `loadConfigFile` out of common include note: this file has lots of smelly bits but it is likely to be rewritten soon anyway so i'm not going to expend a lot of time on it
remove `using namespace std` use ranged for loop remove C-style local variable declarations
remove `using namespace std` use `std::filesystem::path` instead of `char[1024]` for filenames use `ofstream` instead of C stdio combine three identical blocks of code using a lambda fix several instances of using int for size_t remove cruft left over from 40d move local functions into anonymous namespace
remove `using namespace std` move constructors to header replace default destructor with explicitly defaulted destructor push local functions into anonymous namespace some `size_t` cleanups replace `char` array with `std::string`
removed `using namespace std` pushed local functions into anonymous namespace renamed an especially poorly named data type `size_t` adjustments
ab9rf
changed the title
stonesensestonesense, pt. 1
|
Part 1, released for review |
|
oh boy. this will be fun. |
|
though I'm already impressed that the diff is 200 lines negative |
|
initial testing leads to a crash on stonesense load with: [DFHack]# ssense
Stonesense launched
Using allegro version 5.0.10 r1
Invalid filename: `����
[DFHack]#
Thread 11 "dwarfort" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffb58706c0 (LWP 9969)]
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
76 VPCMPEQ (%rdi), %ymm0, %ymm1
(gdb) bt
#0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1 0x00007ffff74ead10 in __printf_buffer (buf=buf@entry=0x7fffb586f190, format=format@entry=0x7fffed51a1cc "Invalid filename: %s\n",
ap=ap@entry=0x7fffb586f298, mode_flags=mode_flags@entry=2) at /usr/src/debug/sys-libs/glibc-2.40-r5/glibc-2.40/stdio-common/vfprintf-process-arg.c:435
#2 0x00007ffff74eb7e9 in __vfprintf_internal (s=0x7fff504a2640, format=0x7fffed51a1cc "Invalid filename: %s\n", ap=0x7fffb586f298, mode_flags=2)
at vfprintf-internal.c:1559
#3 0x00007fffed50d649 in vfprintf (__stream=0x7fff504a2640, __fmt=0x7fffed51a1cc "Invalid filename: %s\n", __ap=0x7fffb586f298)
at /usr/include/bits/stdio2.h:166
#4 LogError (msg=msg@entry=0x7fffed51a1cc "Invalid filename: %s\n") at /home/myk/src/dfhack/plugins/stonesense/main.cpp:101
#5 0x00007fffed4df262 in ContentLoader::parseContentIndexFile (this=this@entry=0x7fff50000ed0, filepath=filesystem::path "stonesense/index.txt" = {...})
at /home/myk/src/dfhack/plugins/stonesense/ContentLoader.cpp:329
#6 0x00007fffed4e146c in ContentLoader::Load (this=0x7fff50000ed0) at /home/myk/src/dfhack/plugins/stonesense/ContentLoader.cpp:214
#7 0x00007fffed4fc2af in reloadPosition () at /home/myk/src/dfhack/plugins/stonesense/MapLoading.cpp:998
#8 0x00007fffed50ee7f in main_loop (display=0x7fff50010a90, queue=0x7fff504ce4a0, main_thread=0x7fffc81f88b0, con=...)
at /home/myk/src/dfhack/plugins/stonesense/main.cpp:309
#9 stonesense_thread (main_thread=0x7fffc81f88b0, parms=<optimized out>) at /home/myk/src/dfhack/plugins/stonesense/main.cpp:573
#10 0x00007fffed06292a in ?? () from hack/liballegro.so.5.0
#11 0x00007fffed09e26e in ?? () from hack/liballegro.so.5.0
#12 0x00007ffff7519f89 in start_thread (arg=<optimized out>) at pthread_create.c:447
#13 0x00007ffff758a4ec in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78in image form (cuz color): I think we need to address the API abuses as part of this cleanup. I'll look at upgrading allegro on Linux to see if that helps too. |
some repeated code was abstracted and a lot of dead code removed |
myk002
approved these changes
Jan 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm going through stonesense and cleaning up/modernizing its code.