Split map fixes into map specific scripts

[Treescrub]

Splitting anv_mapfixes.nut and other map specific fixes into scripts for each map would allow easy creation/modification of fixes for specific maps and would improve readability.

[Tsuey]

Respectfully agree.

I'm still getting the files up to sync with Rayman's pending fixes for Kerry, closed issues and small SirPlease differences, and z_developer_showupdate integration will probably be stable enough for the live game (but still has todo's).

This format should be perfect (avoiding per-campaign, for the unreliability reasons shqke stated):

anv_mapfixes/c1m1_hotel.nut
anv_versus/c1m1_hotel.nut

The other files are pretty low-traffic, I don't think they need splitting (yet).

We should wait to launch this until SirPlease can completely remove their anv_versus.nut and replace it with a anv_customization.nut (to support the multi-map deletions they make).

[Treescrub]

I just finished splitting all the map fixes from anv_mapfixes.nut. I'm working on the map-specific-scripts branch if anyone wants to take a look.
Also I think it would be a good idea to move fixes from anv_maptrigs.nut into the split scripts from anv_mapfixes.nut.

[shqke]

I'm thinking that pathing in a dire need of proper naming, for example we have "anv_" which is not transparent enough - could be given a namespace community.

Just as "fixes" is unlikely to be technically correct, since we add features there too (such as new props for better visuals), - could be replaced with more generic maps.

There's a great deal towards better readability being gained with this change, so it should be possible to merge everything map specific under same file (including c8m4_elevatorfix, anv_versus and others).
It would help to coordinate patches for a specific map across different game modes, namely "versus" and others, and helper functions should improve the look (but that's a different subject).

With these changes, a folder structure could look like:
vscripts/mapspawn.nut
vscripts/community/
vscripts/community/maps/
vscripts/community/maps/BSPNAME.nut
vscripts/community/functions.nut
vscripts/community/z_developer_showupdate.nut

Empty game mode specific cases, such as this can be omitted and their debug messages handled in a parent file.

[Treescrub]

Merging anv_maptrigs and anv_versus into the map specific scripts should be relatively easy to do. I think we might not want to merge c8m4_elevatorfix and c11m5_versus_planecrash because both are roughly 150 lines and it would reduce readability. I think it would be better if those specific fixes were just included as a script in the map script.

anv_tankwarps should be merged too, but it's a bit more complicated because it's called from a tank_spawn event being triggered and needs the handle of the tank to determine whether to warp. Maybe there could be a specific folder for tank warp scripts for each map, and each script has a function that handles the warping.

[shqke]

I see no point to have leftovers like this, it certainly is a map specific code, its size is within a few page scrolls, rest should be covered by multiline comment section which is already written. By placing it within same map file, we make it easier to look for relevant code across a community update fileset we provide, not to mention that having files with uncategorized source code could end up being a bad practice.

As for tank_spawn event, why not catch it outside in parent file and call an optional handler function located at map specific scope file? For consistency it can be put in the beginning of the file, and a practice itself (having individual map specific handlers for any purpose) can be reused.

Only if tankwarps code alone gets bloated as much as average 500 lines per map, then it will impact readability and could use a split.

[Tsuey]

Sometime we'll get Jacob in on this conversation to see what he thinks.

These are the biggest reasons for splitting the files to be map-specific, showing that it's worth while:

• Better collaboration going forward since multiple maps can be worked on at once
• anv_customization.nut can replace the efficiency that only 1-2 high-traffic files had
• In the distant future, BSP lump editing is going to result in a lot of new possibilities/content
• We need organization, especially if we plan on creating filters etc. for this content

A downside worth noting:

• Kerry will now have to diff ~120 files each update, instead of just 1-2

From a productivity point of view, i.e. those months I spent back and forth with Jacob where we only needed to swap 1 file back and forth, the actual worker's productivity is worth a reminder. Empty mode specific fix cases, and even empty mapname.nut placeholders, are useful. Maps that don't have any fixes today may have fixes tomorrow.

Using CTRL+F to find the desired map was easier than having dozens of tabs open, but since it is not the expected convention, and fixes (with current BSP limitations in mind) are 95%+ complete, I agree it's still time to split them. We can just slow-track it.

Derpduck is still working on his ZoneMod Contribution Style Guide. Eventually we should establish similar formatting guidelines, keeping brackets consistent, placeholders are OK, etc.:

https://github.com/Derpduck/L4D2-Comp-Stripper-Rework/wiki/Contribution-Style-Guide

[Tsuey]

Regarding anv_customization.nut:

Let's keep discussion of that here. We might want to consider re-working anv_functions.nut, or whatever necessary, to implement anv_customization.nut first. That can exist without file splits, but file splits should not exist without it.

This file would be successful once it can pre-empt the spawning of entities competitive configs do not want, and would replace their use of modified anv_versus.nut files (which will be outdated with any official update we do).

Comprehensively, those are:

c2m1_highway:

	"_motelskyboxroof_clipinfected"

c2m4_barns:

	"_hittable_bumpera"
	"_hittable_bumperb"

Allowing it to spawn new entities should be trivial, but modifying existing entities either hard or unsupported.

Personally I'd make extensive use of anv_customization.nut for development, since it'd be nice for rapid prototyping before placing stuff into their individual homes.

[shqke]

Using CTRL+F to find the desired map was easier than having dozens of tabs open

Wouldn't worry about it, comments should make process a breeze, as you'll be working with a single map at once.

Empty mode specific fix cases, and even empty mapname.nut placeholders, are useful. Maps that don't have any fixes today may have fixes tomorrow.

Code can always be added again, there's no good reason to keep empty logic, it's not a configuration file with strict requirements afterall. Less code to be sent - less code to track.

Could you please describe use cases of customization code a bit more thorough? I see a feature set in it that I've had in mind, but I'm not familiar with common map issues/patches.

I'd like to take a hit on helper functions code at some point as well.

Also it might be a good time to merge branch with latest update. 🍕

[Tsuey]

Could you please describe use cases of customization code a bit more thorough? I see a feature set in it that I've had in mind, but I'm not familiar with common map issues/patches.

Not sure, honestly. It might be more ideal as a Mutation-style configuration file. All I know is the minimum of what it'd need to do for SirPlease, but it could potentially have a pretty broad feature set, allow specification of filters, add/remove/edit entities.

Implementing anv_customization.nut as a script file would be simple, but also require the user to have some scripting ability. Depending on how elaborate we want to get with future entity/clip/ladder editors etc., that will determine how "customization" could be.

Could also spend months even years thinking about how it "could" be and make no progress, so maybe a script file would be best for now, and it'd resemble the original anv_mapfixes.nut -- only we won't ship the file with the game.

[Treescrub]

Would it be a good idea to have a customization script for each map that runs if the script exists? That would allow customization without relying on exactly how the map specific script is set up, and it wouldn't cause issues with updated scripts unless the customization relies on specific entities the normal script creates. We could also add a variable that can be set to prevent normal map specific scripts from running so that customization scripts can easily work with a blank slate.

Also after looking at anv_tankwarps a bit, it should be fairly easy to merge it by keeping a reference to the loaded map specific script. Then the map specific script could have an optional function that's called whenever a tank warp should happen.

[Tsuey]

Would it be a good idea to have a customization script for each map that runs if the script exists?

Having options is always flat-out appreciated, so that'd be welcome.

Somehow, though, I'd still want to check if some script exists, and if so run it, so we can still use the old (bloaty but arguably efficient) anv_mapfixes.nut switch-case approach. In terms of the BSP hexediting stuff I'm messing around with, I'm already using a modified script for that -- native support would be a relief. It's not much content, but keeping it all in one place (especially with the pre-anticipation we may one day trash it) keeps things clean for me.

So far @Treescrub, the branch is looking good, and thanks for getting the anv_versus stuff into anv_mapfixes -- that's one big benefit of the split I did not mention... much cleaner.

[Treescrub]

A main customization script that's always run is definitely a must. If there was no global customization script, then modders would have to edit anv_mapfixes to do mode specific changes.

I think the only script left that needs to get split is anv_tankwarps, once I get that done we'll need to thoroughly review and test everything.

[Treescrub]

I think we should make customization scripts a separate issue here on GitHub which we can work on once every script is split. Discussing it on this issue is a bit confusing and hard to follow.

[Tsuey]

I think we should make customization scripts a separate issue here on GitHub which we can work on once every script is split. Discussing it on this issue is a bit confusing and hard to follow.

Sure, I have no problem and whatever works. That is the priority imo, the split is beneficial long-term and is an invisible change to players (with complex implications if something goes wrong), but customization could be immediately put to use by SirPlease. Maybe another branch just for it.

Specifically, they should not have to update this file and re-apply their changes every time we update the live game:

https://github.com/SirPlease/L4D2-Competitive-Rework/blob/master/scripts/vscripts/anv_versus.nut

As for thorough review, I was thinking (especially if reconciling differences becomes too much) I could follow your split example and make sure we both end up with the same files. The testing will take a some time, though.

Also @shqke might start a PR (or preferably a branch? I'm new here hey), for the idea he mentioned.

[shqke]

We should be able to freely merge every completed change, - versions can be tagged and newer tags to be pushed for update with delay, unless confirmation is required from Valve on every proposed change.

I'll gladly make a PR, once these changes are merged as merging with new path names can get messy (if there is more added to the base branch).

[Tsuey]

unless confirmation is required from Valve on every proposed change.

I did clear the idea of splitting to individual map scripts with Kerry and he's OK with it, it'll just take him extra time to verify everything after that final commit. Splitting the files, in combination with new fixes that may not be live for weeks from now, I'm going to assume is a bit unreasonably chaotic, so player-oriented fixes come first.

All files are manually diffed and verified for each update and it's one of the reasons ShowUpdate() was created, so the changes can be easily reviewed. The only time we forgot to diff the changes was November 4, 2020, when this happened (meaning, it'll never happen again, this caused a panicked 12-hour disturbance, we fixed it quickly though):

An update has been released for Left 4 Dead 2. - Fixed a regression that caused map exploit fixes to appear in co-op modes again.

https://www.l4d.com/blog/post.php?id=77415

[Tsuey]

Might be worth mentioning that infodecals:

https://github.com/Tsuey/L4D2-Community-Update/blob/master/scripts/vscripts/mapspawn.nut#L70

Would be nice to split into the individual map script files, but afaik would require some technical voodoo to get them to spawn early enough. Could think about transferring them to LMP files someday.

[Treescrub]

Might be worth mentioning that infodecals:

https://github.com/Tsuey/L4D2-Community-Update/blob/master/scripts/vscripts/mapspawn.nut#L70

Would be nice to split into the individual map script files, but afaik would require some technical voodoo to get them to spawn early enough. Could think about transferring them to LMP files someday.

I think it could be done by including the map specific script in mapspawn, and just wrapping everything in functions so that anv_mapfixes can still call the map specific non-decal fixes without including the script again.

[Treescrub]

It looks like anv_mapfixes is included from mapspawn, so in mapspawn the map specific script can be included, and a function (something like DoMapSpawnFixes) can be called to be able to do stuff when the map spawns. All the current code for the map specific scripts would have to be put into a new function (something like DoPostSpawnFixes) so that mapspawn won't run non-decal fixes.

[Treescrub]

I just finished splitting everything into map specific scripts, so we just need to do testing now to make sure it works properly. We'll need to make sure decals still work, general map fixes still work (need to check second round in versus), and tank warps work.

It should be as easy as downloading the files on the branch as a ZIP, dragging the scripts folder into the left4dead2 folder, and checking the maps (or the changed scripts could be put into an addon).

Also I think we should comment on this issue with the maps checked so that we don't miss any maps.

[Treescrub]

I just tested it a bit. Looks like all the scripts run properly in versus for every map (after I fixed a global variable not being global), the map specific script DoRoundFixes function is ran each round, and the map specific script DoMapSpawnFixes function is ran at mapspawn. I think just testing survival and tank warps should be enough now.

[Tsuey]

Looks good to me so far and you might want to start a PR.

@jacob404 may want to confirm the Tank warp fixes still work, too.

Also, it'll take me a few days, but we'll need to have someone join a Dedicated Server, leave, then rejoin to ensure the infodecals are still there. Easiest on c8m5. May take me a few days to get to that unless someone else beats me to it. (Alternatively, they could just join late, as in after other players have left the safe room -- though I never tested it that way.)

[jacob404]

@jacob404 may want to confirm the Tank warp fixes still work, too.

Just tested and can confirm tank warps are working as expected