Queue updates / refactor #262
Conversation
|
All tests are running green for my newly added methods. Next step is to remove the old methods and then rename the new methods to the old methods. |
|
The old C code was pretty fast and loose with how it handled parameters. (So many void pointers. So many.) This new C++ code needs real types in order to do it's dispatch. Therefore, I'm making wrappers for methods that are placed on the queue. These wrappers basically take in the C++ parameters and calls the old method the C way. Ex: void act(const char *format, CHAR_DATA *ch, const void *arg1, const void *arg2, int type)
{
act_new(format, ch, arg1, arg2, type, POS_RESTING);
}
void act_queue(std::string format, CHAR_DATA *ch, OBJ_DATA *arg1, CHAR_DATA *arg2, int type)
{
act_new(format.c_str(), ch, (void*)arg1, (void*)arg2, type, POS_RESTING);
} |
|
Latest commit has everything switched over to using the new queue methods. Next step is to rename the new methods back to the old names. Eg. RS.Queue.AddToNewQueue(...)
...to RS.Queue.AddToQueue(...)
...After that is complete, I'm going to have to run some extensive testing to ensure that the strict typing didn't break anything or any weird edge cases pop up. |
|
Renamed the new methods back to the old names. Removed the queue.c file and its entry in the CMake file. Goodbye old queue. You valiant warrior. You magical black box of void pointers. Thank you for your service, friend. |
…ld cause undefined behavior. Added a few more queue compliant functions
|
Good news, it's almost perfect! Bad news, we've got a bug. When creating a new character, an assistant npc is assigned to the character. That mob speaks to the player frequently to give out good info that's useful to the player. That mob however doesn't speak now - he just does this: � ^�\ Sample: |
|
Looking at it further, not all NPC speech is broken - NPCs sending green text work (vendors speaking to you) and some NPCs using yellow text (guards using say in front of you) and I can see my own text when I use |
|
That's weird. Thank you for testing it out. Looks like I may have missed something or exposed something else. At any rate, I've got a starting point. |
Yeah this is great so far! I found another (probably related) issue: I cannot quit the game, because it says there are items pending for me. I'd bet it's queued up items from the assistant that need to be force removed. |
|
There was this weird thing where they were putting library calls in the queue ex: RS.Queue.AddToQueue(6, do_tell, mob, buf);
RS.Queue.AddToQueue(6, sprintf, buf, (char *)"%s I see you, mortal.", ch->name);
changed to
buffer = fmt::format("{} I see you, mortal.", ch->name);
RS.Queue.AddToQueue(6, do_tell_queue, mob, buffer); another ex: temp = palloc_string(row[0]);
RS.Queue.AddToQueue(inc * 5, free, temp);
RS.Queue.AddToQueue(inc * 5, say_to, fat, ch, (char *)"Just do yourself a favor, and watch out for $t.", temp);
changed to
temp = std::string(row[0]);
RS.Queue.AddToQueue(inc * 5, say_to_queue, fat, ch, "Just do yourself a favor, and watch out for $t.", temp);I wonder if that has something to do with printing uninitialized / junk memory. |
|
So this piece of text comes from act("You walk through $p and find yourself somewhere else...", ch, portal, nullptr, TO_CHAR);This piece of text looks to be from /* Greet trigger for mobs */
if (is_npc(fch) && IS_SET(fch->progtypes, MPROG_GREET))
fch->pIndexData->mprogs->greet_prog(fch, ch);The greet program in question comes from void greet_prog_opening_greet(CHAR_DATA *mob, CHAR_DATA *ch)Haven't found any queue related functions around those parts yet. Still looking. |
|
I found the code for the assistant (referenced in code as 'academy pet' or 'pet') in Looking at the text when I create a character, none of these become visible to the player, so I'm not sure which piece is broken. |
|
Hmmm, I wonder if it's having trouble with the multi-line string syntax. |
|
The last line in that function looks troubling. It's putting using It's also using one of those weird homemade string allocators. |
|
Unfortunately that didn't fix it, but it did cause me to look further into the problem - the pets may be having issues because they're not even fully generating in the player file. I checked both of the players, and maybe it's because it's not being fully composed. When it's being added to the player file now (it's stored in the bottom of the player file typically) it's not there - the NPC isn't being fully composed now for some reason. A properly composed academy pet at the end of the player file: The end of a new player after the upgrade: |
|
PET information is written to the player file by
|
|
Making a bit of headway. It's processing queue events now, not segfaulting and I can exit. Still an issue with passing some parameters. server: client |
|
With this latest commit, I believe I have all the bugs ironed out. Tested it locally and game seems to be acting like it should. Going to remove the WIP tag and let you all do a proper review. Side note: I've definitely learned a lot more about C++ debugging this PR. Maybe learned too much ;) Thanks for putting up with me while working through this. |
|
Looking now! |
DO NOT MERGE!!!Seriously do not merge.I'm wanting to get some feedback on some queue refactoring that I'm doing. It compiles, but that's all I can guarantee atm. It'll probably segfault during runtime. I need to write a slew of tests and actual run it to verify.Functions to take a look at in
queue.h. These names will eventually be changed to their old named counterparts. Just for testing / compiling purposes :)All the old functions are still there to take a look and compare. Most of the new function parameters are unchanged except for adding to queue:
old way
RS.Queue.AddToNewQueue(speech->current_line->delay, 3, speech_handler, ch, mob, speech);It took a timer, the number of arguments to the function, the function, and the arguments. It was limited on the number of function arguments that can be passed to it (max of 8).
new way
It takes a timer, the function, and the arguments. It is unlimited on the number of function arguments that can be passed to it.
Anyways, I'm looking for comments/suggestions. Do those methods look like an improvement over the old or not?