Making this an object had little to no advantage, except being able to remove the deleter code. Now, with some of the class data already being allocated in a memory arena so that freeing it is easier, this can also be used for the drop item lists which makes it unnecessary to subject them to the GC. This also merges the memory arenas for VM functions and flat pointers because both get deleted at the same time so they can share the same one.
As it stood, just compiling the internal ZScript code created more than 9000 DObjects, none of which really need to be subjected to garbage collection, aside from allowing lazy deallocation.
This puts an incredible drag on the garbage collector which often needs several minutes to finish processing before actual deletion can start.
The VM functions with roughly 1800 of these objects were by far the easiest to refactor so they are now. They also use a memory arena now which significantly reduces their memory footprint.
- use a memory arena to store flat pointers so that the messed up cleanup can be avoided by deallocating this in bulk.
- added a new SO opcode to the VM to execute a write barrier. This is necessary for all objects that are not linked into one global table, i.e. everything except thinkers and class types.
- always use the cheaper LOS opcode for reading pointers to classes and defaults because these cannot be destroyed during normal operation.
- removed the pointless validation from String.Mid. If the values are read as unsigned the internal validation of FString::Mid will automatically ensure proper results.
error: use of undeclared identifier 'op'
error: no matching function for call to 'ListEnd'
error: no matching function for call to 'ListGetInt'
error: no matching function for call to 'ListGetDouble'
...
This isn't done for register based variables so for consistency it should not be done for stack based variables, too.
This difference in handling made it impossible to check the target of a hitscan attack if it was destroyed by getting damaged.
If a later module reused an existing name for a different class or struct type, this new name would completely shadow the old one, even in the base files.
Changed it so that each compilation unit (i.e. each ZScript and DECORATE lump) get their own symbol table and can only see the symbol tables that got defined in lower numbered resource files so that later definitions do not pollute the available list of symbols when running the compiler backend and code generator - which happens after everything has been parsed.
Another effect of this is that a mod that reuses the name of an internal global constant will only see its own constant, again reducing the risk of potential errors in case the internal definitions add some new values.
Global constants are still discouraged from being used because what this does not and can not handle is the case that a mod defines a global constant with the same name as a class variable. In such a case the class variable will always take precedence for code inside that class.
Note that the internal struct String had to be renamed for this because the stricter checks did not let the type String pass on the left side of a '.' anymore.
- made PEnum inherit from PInt and not from PNamedType.
The old inheritance broke nearly every check for integer compatibility in the compiler, so this hopefully leads to a working enum implementation.
- fixed the return type checks in CallStateChain. These made some bogus assumptions about what return prototypes to support and would have skipped any multi-return function whose first argument was actually usable.
- improved the class pointer to string cast to print the actual type it describes and not the class pointer's own type.
- fixed: The 'is' operator created non-working code when checking the inheritance of a class pointer, it only worked for objects.
There are a few which require explicit native construction or destruction that need to be exported to the VM, e.g. FCheckPosition.
The VM cannot handle this directly, it needs two special functions to be attached to handle such elements.
- made ModifyDamage calls iterative instead of recursive. With going through the VM they'd be too costly otherwise.
- small optimization: Detect empty VM functions right when entering the VM and shortcut them. This is to reduce the overhead of virtual placeholders, which in a few cases (e.g. CanCollideWith and ModifyDamage) can be called quite frequently.
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.
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.
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 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.
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.
Needless to say, this is simply too volatile and would require constant active maintenance, not to mention a huge amount of work up front to get going.
It also hid a nasty problem with the Destroy method. Due to the way the garbage collector works, Destroy cannot be exposed to scripts as-is. It may be called from scripts but it may not be overridden from scripts because the garbage collector can call this function after all data needed for calling a scripted override has already been destroyed because if that data is also being collected there is no guarantee that proper order of destruction is observed. So for now Destroy is just a normal native method to scripted classes
- scriptified all Effect functions of Fastprojectile's children
- implemented access to class meta data.
- added a VM instruction to retrieve the class metadata, to eliminate the overhead of the function call that would otherwise be needed.
- made GetClass() a builtin so that it can use the new instruction
Important note about this commit: Scriptifying CFlameMissile::Effect revealed a problem with the virtual function interface: In order to work, this needs to be explicitly enabled for each single native class that may be used as a base for a scripted class. Needless to say, this will end up way too much work, as there are over 100 native classes, excluding those which will be scriptified. But in order to fix the problem this partially broken state needs to be committed first.
- fixed: FxAssignSelf did not the correct number of registers for vector operations.
- fixed a few asserts in vector2 instructions.
- turned the virtual AActor::HitFloor method into a flag MF7_SMASHABLE. The only use of this function was to kill Hexen's pottery when they hit the floor, and this looks like something that can be exposed to modders less clumsily.
- added new VARF_Transient flag so that the decision whether to serialize a field does not depend solely on its native status. It may actually make a lot of sense to use the auto-serializer for native fields, too, as this would eliminate a lot of maintenance code.
- defined (u)int8/16 as aliases to the byte and short types (Can't we not just get rid of this naming convention already...?)
- exporting the fields of Actor revealed a few name clashes between them and some global types, so Actor.Sector was renamed to CurSector and Actor.Inventory was renamed to Actor.Inv.
- fixed: The code generator had no good safeguards for exceeding the maximum amount of registers.
All there was was a handful of pitiful asserts which in production code do nothing at all but generate broken output.
Even worse, the VM was hardwired to at most 255 constants per type per function by storing the constant count in a byte! This has been extended to 65535, but since many instructions only have a byte available for the constant index, a workaround had to be added to do a two-instruction setup if larger indices are needed.
- added new VM instructions to access the constant tables with a variable index.
- refactored VMFunctionBuilder's constant tables so that they are not limited to one entry per value. While this works fine for single values, it makes it impossible to store constant arrays in here.
- implemented method calls from struct instances.
- optimized disassembly of VM call instructions to print the function's name at the end where it is more visible and does not need to be truncated. Also use the printable name for script functions here.
- made APlayerPawn::PlayAttacking(2) virtual script functions so that mods have better control over player animations. Note that these have no native base so they skip the templated interface for managing virtual functions.
- changed Dehacked weapon function lookup to check the symbol table instead of directly referencing the VM functions. Once scriptified these pointers will no longer be available.
- removed all special ATAGs from the VM. While well intentioned any pointer tagged with them is basically unusable because it'd trigger asserts all over the place.
- scriptified A_Punch for testing pass-by-reference parameters and stack variables.
- gave OP_CONCAT some sane semantics. The way this was defined, by specifying the source operands as a range of registers instead of a pair like everything else made it completely useless for the task at hand.
- changed formatting for floats to %.5f which for normal output in a game makes more sense. For special cases there should be a special formatting function for ints and floats that can do more specialized conversions.
- make the pointer to string cast a bit more useful by using the actual object's type rather than 'Object' which can be a great asset when debugging.
- fixed a few bad asserts.
- fixed code generation for using local variables as array index. This must use a different register for the array element offset because the original register may not be overwritten.
- instead add a list of SpecialInits to VMScriptFunction so this can be done transparently when setting up and popping the stack frame. The only drawback is that this requires permanent allocation of stack objects for the entire lifetime of a function but this is a relatively small tradeoff for significantly reduced maintenance work throughout.
- removed most #include "vm.h", because nearly all files already pull this in through dobject.h.
- fixed several places in the code generator that did not consider locked registers for local variables: array indices, abs and floating point builtin functions.
- added some debug aids to the bounds opcode. Just triggering an exception here which loses all relevant info is perfectly useless in a debug situation.
- added a DActorIterator class.
- fixed: It was not possible to have functions of the same name in two different classes because the name they were searched for was not qualified by the class. Changed so that the class name is included now, but to avoid renaming several hundreds of functions all at once, if the search fails, it will repeat with 'Actor' as class name.
This commit contains preparations for scriptifying Hexen's Dragon, but that doesn't work yet so it's not included.
This could cause problems with functions that take states as parameters but use them to set them internally instead of passing them through the A_Jump interface back to the caller, like A_Chase or A_LookEx.
This required some quite significant refactoring because the entire state resolution logic had been baked into the compiler which turned out to be a major maintenance problem.
Fixed this by adding a new builtin type 'statelabel'. This is an opaque identifier representing a state, with the actual data either directly encoded into the number for single label state or an index into a state information table.
The state resolution is now the task of the called function as it should always have remained. Note, that this required giving back the 'action' qualifier to most state jumping functions.
- refactored most A_Jump checkers to a two stage setup with a pure checker that returns a boolean and a scripted A_Jump wrapper, for some simpler checks the checker function was entirely omitted and calculated inline in the A_Jump function. It is strongly recommended to use the boolean checkers unless using an inline function invocation in a state as they lead to vastly clearer code and offer more flexibility.
- let Min() and Max() use the OP_MIN and OP_MAX opcodes. Although these were present, these function were implemented using some grossly inefficient branching tests.
- the DECORATE 'state' cast kludge will now actually call ResolveState because a state label is not a state and needs conversion.
Now even in DECORATE it is possible to report most cases in which user variables are accessed from non-item states as an error. This will report all states which can be traced by a direct link from a special state label. It will not find states that only get used via A_Jump etc.
Syntax-wise I chose to make it as strict as possible to reduce the chance of errors: Virtual base functions must be declared with the 'virtual' keyword, and overrides in child classes with the 'override' keyword. This way any mismatch in parameters that otherwise would cause silent failure will outright produce a compile error.
- Since the number of small allocations here is extremely high this will help a lot to prevent fragmentation and since most nodes are collected up front and this is done when no large resources are being loaded it won't cause heap spikes.
let Emit methods delete FxExpression arrays when they are done.
- For some reason the deletion process does not work 100%, there are always some nodes left behind and so far I haven't found them. This ensures that these arrays do not live any longer than needed.
* use the function build list instead of the function to pass the info. The function is permanent so not the best place for compile-time info.
* pass along the current state index which is needed to calculate the target state.
- made some tests about calling script code from native functions.
* scriptified A_SkullAttack to have something to test
* changed the A_SkullAttack call in A_PainShootSkull.
* use a macro to declare the function pointer. Using local static variable init directly results in hideous code for the need of being thread-safe (which, even if the engine was made multithreaded is not needed here.)
* Importsnt node here: Apparently passing an actor pointer to the VMValue constructor results in the void * version being called, not the DObject * version.
- fixed: The state index comparison against 0 was broken.
- fixed: Resolving codegen nodes must set the strictness flag per function so that ZSCRIPT and DECORATE are done properly.
