SnowyMouse / invader

Free toolkit for Halo: Combat Evolved for creating maps and assets
https://invader.opencarnage.net
Other
57 stars 9 forks source link

[invader-build] Compile all scripts in the scenario tag upon building #80

Closed SnowyMouse closed 2 years ago

SnowyMouse commented 4 years ago

First, invader-build MUST recompile scripts in the scenario tag upon building. Re-compiling the scripts in the scenario data can be useful when building cache files, as this can help prevent potential undefined behavior and crashes, allowing the user to correct potential unforeseen errors. Invader, lately, has had plenty of error checking added to it, so this is only natural that we check the scripts, too.

For example, some maps contain scripts that crash the retail and demo Halo client because they contain commands that are exclusive to Halo Custom Edition such as sv_say or multiplayer_draw_teammate_names, so if a map is being built for retail and the script compiler detects invalid commands being used, it should error. This is why some maps (e.g. Coldsnap) will crash if run outside of Halo Custom Edition.

Second, invader-build should also reject scenario tags that are missing source data and contain scripts. This idea may seem insane at first since most extracted scenario tags do NOT have scripts. However, such tag files are NOT a product of normal usage of the Halo Editing Kit. You will never encounter a scenario tag that has scripts but has no source data unless it was generated (i.e. extracted) by a tool outside of the Halo Editing Kit.

Also, we cannot safely assume what engine the script syntax data targets. This is due to the fact that script commands are compiled into indices as an optimization for Halo's script handler. In MOST cases, scripts will seemingly work just fine even if used in the wrong version. However, it is often when things are seemingly working is when the most serious of problems occur.

In fact, since Sapien has commands that Halo Custom Edition doesn't have (e.g. debug_pvs and radiosity_*), we cannot even trust Sapien-compiled scripts even if it is unlikely that they would normally be problematic. Therefore, scripts made for Custom Edition must be compiled against Halo Custom Edition, not Sapien, to be sure that the map will work.

SnowyMouse commented 4 years ago

I've updated the original message to better detail why this is important.

SnowyMouse commented 4 years ago

It looks like tool.exe recompiles scripts if the scripts block is empty based on the source file data. It also regenerates the References array, too. It does not recompile scripts if the scripts block is not empty, but it does regenerate the References array.

It seems the behavior is this:

  1. If the scenario has no scripts/globals, check if there is source data. If so, compile the scripts in the source data, thus adding scripts/globals.
  2. If the scenario has scripts/globals, regenerate the references array based on the script syntax data.

This means that, if there is no source data, then the script data is left unmodified. We probably shouldn't trust this, as, again, you cannot make a scenario file that has scripts but no source data using the Halo Editing Kit, and how such tags are handled is probably not defined by Bungie/Gearbox when tool.exe build-cache-file was made. Therefore, it is better to not allow such maps to build.

It is likely that, if and when invader-build is eventually used by the current Halo Custom Edition modding community, users may complain about this behavior when using tags extracted without the source data regenerated, since most tag extractors do not do this by default, if at all, due to this oversight in tool.exe. What they should really do is complain to the developer of their tag extraction tool for outputting broken tags, instead.

The point of the invader-build rewrite is to catch errors with tags that can cause undefined behavior with the map when being played in the game, and since these scenario tags are technically undefined by the original game developers due to their incompleteness, these tags are technically broken by our standards. Therefore, these tags should NOT be used for building cache files, thus invader-build is working correctly.

So, tools that extract scenario tags with scripts without generating source data are producing technically broken tag files. Therefore, such tools need to be fixed, as this is clearly a bug.

SnowyMouse commented 4 years ago

tool.exe also will recompile scripts if using child scenarios. This is likely why some maps' scripts won't compile in Sapien due to duplicate references but exist nevertheless in Halo.

SnowyMouse commented 2 years ago

Done as of 7c544fb062dbf271e8efbc78481968b11305b4d9