That only leaves the Scale function which is still being used in a few places and which would create considerably worse code without assembly on 32 bit platforms. This is also far too primitive (2 or 3 assembly instructions) to claim any copyright on it, so I think m_fixed.h can now be considered free of Build-related issues. The deficated inline headers have been removed because that sole remaining function could be easily moved into m_fixed.h.
Instead of trying to fix Simplify, which seems to be a lost cause, the ring list now gets unraveled into an array which is immune from this type of problem.
- renamed all functions in r_walldraw.cpp to give them names more in line with Doom's naming conventions. Since this is not Build code anymore it also shouldn't use Build names to avoid giving a false impression.
- cleaned out a lot the SafeDivScale stuff in m_fixed.h. The only SafeDivScale variant still in use was #16 for FixedDiv, so all the SafeDivScale stuff has been removed and the 16 variant renamed to FixedDiv because that's the only form in which it is still being used. (2x in R_DrawVoxel and 1x in ACS's FixedDiv PCode.)
- removed Build notice from m_fixed.h because aside from the inlines includes there is nothing here from Build anymore.
This was used in only 4 places, 3 of which could easily be replaced with a memset, and the fourth, in the Strife status bar, suffering from a pointless performance optimization, rendering the code unreadable - the code spent here per frame is utterly insignificant so clarity should win here.
- removed the Build license note from r_bsp.cpp.
This note was for code in R_AddLine which had been both refactored into several subfunctions and completely replaced with a floating point version. What is left is just some basic common math without any traits that resemble anything in Build.
Because every bit of Build code that can be removed is a good thing.
This was only used in two places, one of which could be done better, the other one in the voxel drawer setup now uses a local C-inline version.
There's two restrictions, though:
* on one-sided-line portals fog boundaries will not be drawn.
* the filler sector behind the portal may not have a sky ceiling texture. This is because the drawing code contains several sky checks which get in the way here.
Some benchmarking shows that on SSE systems it only harms performance and compared to the intrinsics version the gains are too marginal for something this infrequently called.
Doing 100000 calls of DoBlending results in a 5 ms decrease of using assembly vs intrinsics on a 3.4 GHz Core i7, meaning that even on a computer that is 10x slower you can still do 1000 or so blends per frame without a speed hit.
For most attack functions this is wrong, it's only the Hexen fighter attack needing this particular value, so it has been split up into two return values now.
This can see some heavy use in iterators where saving several hundreds of function calls can be achieved. In these cases, using a function to do the job will become a significant time waster.
On modern systems it is actually slower than the C version, only on old ones it is marginally faster - but the overall execution time for this function is so low that even in the worst case scenario the minor loss of performance on older systems is still not relevant.
This will get called for both actors taking part in a collision, if one of the two calls returns false it will immediately abort PIT_CheckThing with no collision taking place at all.
* It will now use #include, just like most other definition formats and can be mixed with regular definitions. However, due to how the Lemon-generated parser works this will not recursively pull in all files, but store them in a list and process them sequentially. Functionally this shouldn't make a difference, because ZScript is mostly order-independent - the only thing where order is important is native classes, but these are completely internal to zdoom.pk3 where proper order is observed.
Note that this only applies if both are in the same block. Just like in C++, it is perfectly legal to have the same variable name in two different nested scopes.
The original implementation just printed a mostly information-free message and then went on as if nothing has happened, making it ridiculously easy to write broken code and release it. Changed it to:
* Any VMAbortException will now terminate the game session and go back to the console.
* It will also print a VM stack trace with all open functions, including source file and line numbers pointing to the problem spots. For this the relevant information had to be added to the VMScriptFunction class.
An interesting effect here was that just throwing the exception object increased the VM's Exec function's stack size from 900 bytes to 70kb, because the compiler allocates a separate local buffer for every single instance of the exception object.
The obvious solution was to put this part into a subfunction so that it won't pollute the Exec function's own stack frame. Interesting side effect of this: Exec's stack requirement went down from 900 bytes to 600 bytes. This is still on the high side but already a lot better.
- fixed PARAM_ACTION_PROLOGUE to assign correct types to the implicit pointers. It gave the actual class to the wrong one, which until now did not matter because all functions were using 'Actor', regardless of actual class association.
- fixed the definition of IceChunk and removed some redundant code here. Since A_FreezeDeathChunks already calls SetState, which in turn calls the state's action function, there is no need to call it again explicitly.
- throw a useful exception when a VM abort occurs, the simple enum was incapable of reporting anything more than the barest minimum, which at least for array index out of bounds errors was insufficient.
The current exception mechanism is still insufficient. It really has to report a proper crash location and print a stack trace to the maximum extent possible. Instead it just prints a message and happily goes on. This is not a good solution.
Although this already helps a lot with the messed up code generated for comparisons it's not really a solution for this - it still needs a proper implementation to generate efficient code.
- fixed: When replacing a tentative class, the pointers in the morph objects were not replaced. Instead of adding more ReplaceClassRef methods I chose to integrate this part into the PointerSubstitution mechanism and delete ReplaceClassRef entirely. The code had some oversights anyway that would have caused problems, now that non-actors can be created.
It is utterly pointless to require every function that wants to make a VM call to allocate a new stack first. The allocation overhead doubles the time to set up the call.
With one stack, previously allocated memory can be reused. The only important thing is, if this ever gets used in a multithreaded environment to have the stack being declared as thread_local, although for ZDoom this is of no consequence.
- eliminated all cases where native code was calling other native code through the VM interface. After scriptifying the game code, only 5 places were left which were quickly eliminated. This was mostly to ensure that the native VM function parameters do not need to be propagated further than absolutely necessary.
- added call wrappers and script hooks for all relevant virtuals in AInventory.
- made GetSpeedFactor and GetNoTeleportFreeze entirely scripted because they are too trivial - also do them iteratively, just like HandlePickup, because it's just a better way to do this stuff.
- moved health items to their own file.
- scriptified ScoreItem and MapRevealer whose entire functionality was a small TryPickup method.
- fixed: bit fields in global variables were not correctly written.
This should conclude the inventory cleanup. It is now possible again to find things in there.
It's about time this stuff is getting cleaned up seriously. Both a_pickups.cpp and a_artifacts.cpp are so overstuffed that it has become a chore finding stuff in there.
- removed the native parts of SpecialBlastHandling. Since this is called from the script side and the only remaining native remnant was an empty function it's now 100% scripted.
- cleaned up the virtual function interface of APlayerPawn which still had many virtual declarations from old times when class properties were handled through virtual overrides. None of this makes sense these days anymore.
- Changed the glass shards so that they do not have to override FloorBounceMissile. It was the only place where this was virtually overridden and provided little usefulness.
- made 'out' variables work.
- fixed virtual call handling for HandlePickup.
- added a String class to allow attaching methods to the builtin string type. This works by checking if the left side of the member accessor is a string and just replacing the tyoe in this one place, all the rest is automatic.
- merged the FrontBlock searcher for the Bloodscourge into RoughMonsterSearch. This also fixes the bug that the searcher was not initialized properly for the MageBoss.
- made '->' a single token. Although ZScript does not use it, the parser tends to get confused and fatally chokes on leftover arrows so this ensures more robust error handling.
A few notes:
* this accesses the lines array in sector_t which effectively is a pointer to an array of pointers - a type the parser can not represent. The compiler has no problems with it, so for now it is defined internally.
* array sizes were limited to 65536 entries because the 'bound' instruction only existed as an immediate version with no provisions for larger values. For the static map arrays 65536 is not sufficient so now there are alternative instructions for these cases.
* despite the above, at the moment there is no proper bounds checking for arrays that have no fixed size. To do this, a lot more work is needed. The type system as-is is not prepared for such a scenario.
- fixed: Assignment from a readonly to a read-allowed pointer must be an error.
- made GetDefaultByType a builtin so that it can do proper type assignment to the result, which for a function would be problematic in this case, even if automatic type deduction was implemented. Since this returns the class defaults which are not a real object, the result cannot be subjected to a type cast.
- error out if a type cast of a readonly pointer is attempted.
- fixed: FxBooleanNot could clobber a local variable because it used the source register to manipulate the result.
- fixed issues with the refactoring of the recent commits. This one starts again.
- added builtins for TextureID.
Note about builtins: Currently they are just hacked into the compiler backend. They really should be made part of the respective types to keep matters clean and allow more widespread use of builtins to create more efficient code.
- refactored the ModifyDamage interface to be more scripting friendly.
In general it should be avoided having to call directly into chained inventory functions because they are very problematic and prone to errors. So this got wrapped into a single handler (on AActor, not AInventory!) which will later make it easier to refactor the parameters of ModifyDamage to work better for scripting and avoid the chaining.
Two reasons for this:
1. if this has to be routed through the VM each recursion will cost 1000 bytes of stack space which simply is not good.
2. having the virtual function only care about the item itself but not the entire inventory chain is a lot less error prone for scripting.
Since the scripting interface needs a separate caller function anyway this seemed like a good time to change it. The same will be done for the other chained inventory handlers as well.
- allow switch/case with names.
- fixed break jump target handling for switch/case. This only worked when the break was in the outermost compound statement, those in inner ones were missed.
Interesting tidbit: The damage calculation in P_MinotaurSlam had been incorrect for the Heretic version since the friendly Hexen Dark Servant was added, but nobody ever noticed in 14 years...
