search

mbbsemu / MBBSEmu

The MajorBBS Emulation Project is an Open Source, Cross-Platform emulator for easily running The MajorBBS & Worldgroup Modules
https://www.mbbsemu.com
MIT License
132 stars 14 forks source link

[WCCMMUD] MajorMud - Reading a spell scroll crashes the system #207

Closed paladine closed 3 years ago

paladine commented 3 years ago

Module Information WCCMMUD

Describe the bug I created a Paladin, bought two spell scrolls. I read the first which seemed to work, though there was some sort of prfmsg shortening. The second scroll I read crashed the system

To Reproduce See above

Expected behavior I learn the spell.

Unhandled exception. System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at MBBSEmu.Memory.MemoryCore.GetByte(UInt16 segment, UInt16 offset) in /home/tcj/src/MBBSEmu/MBBSEmu/Memory/MemoryCore.cs:line 307
   at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.Execute(IntPtr16 entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassState, Queue`1 initialStackValues, UInt16 initialStackPointer) in /home/tcj/src/MBBSEmu/MBBSEmu/HostProcess/ExecutionUnits/ExecutionUnit.cs:line 105
   at MBBSEmu.Module.MbbsModule.Execute(IntPtr16 entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassSetState, Queue`1 initialStackValues, UInt16 initialStackPointer) in /home/tcj/src/MBBSEmu/MBBSEmu/Module/MBBSModule.cs:line 259
   at MBBSEmu.HostProcess.MbbsHost.WorkerThread() in /home/tcj/src/MBBSEmu/MBBSEmu/HostProcess/MbbsHost.cs:line 300
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location where exception was thrown ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()
enusbaum commented 3 years ago

Couple issues here that prevented spells from being read.

skpwrd was returning Registers.SetPointer(stringPointerBase + i);, where i was defined in the for loop as var i = stringPointerBase.Offset. So basically it was returning offset + offset + delta. 👎

I fixed skpwrd to return the correct pointer.

The next issue was lastwd was establishing a new buffer and returning a pointer to another string, where the documentation states it returns a pointer to the string passed in:

image

I updated lastwd to return .LastIndexOf(' ') and add that to the offset (handling -1 not found by adding 1, pointing to the start of the string).

I'm writing Unit Tests for both of these methods and I'll get the PR in.

image