search

rezalas / riftshadow

Dedicated to the preservation of the game and modernization of a classic mud codebase.
MIT License
18 stars 14 forks source link

Queue semantics #206

Open sean-gilliam opened 1 year ago

sean-gilliam commented 1 year ago

Looking at the code to add a node to the internal queue has some weirdness to it. If you look at the below code, it appears to have stack semantics instead of queue semantics. eg. the code looks to be LIFO instead of FIFO.

The code creates a new node. Makes the queue_first (head) pointer to be the next node element in the queue. Afterwards, it makes the new node the queue_first (head) pointer. Not sure if the codebase relies on this behavior or not. If it does, then the CQueue moniker is misleading and should be CStack or similar. If not, then we should probably fix it to follow queue semantics.

https://github.com/rezalas/riftshadow/blob/3fcf9b9503f91da4de5136f12051acd2ede57d7b/code/queue.c#L71-L75

rezalas commented 1 year ago

I won't lie, I don't understand his particular area well enough to give an educated opinion yet. I know when we had to fix it the problem was everything executed backwards from what it was expecting and it was messing up command sequences. That said, I believe execution of actions was intended to be FIFO, as the first to execute an ability was granted the action. So if it would have an effect on that, it would be good to correct it.

sean-gilliam commented 1 year ago

Good point.