wraitii Mon, May 9, 8:13 PM
- Differential Revision
- D4630: Fix map errors from GetTemplate not being available in the sim
- rP26866: Fix crash in non-visual autostart from not loading terrain textures
- Build Status
Buildable 20279 Build 48772: Trigger Windows Autobuild Build 48771: Post-Commit Build Jenkins Build 48770: Post-Commit Build (macOS) Jenkins
[16:38:28] <elexis> try this [16:38:28] <elexis> 1. revert 26867 [16:38:28] <elexis> 2. add an argument GetTemplate to loadCivFiles and replace Engine.GetTemplate by GetTemplate [16:38:28] <elexis> 3. pass Engine.GetTemplate in all calls, except the one call in the simulation scripts (survival of the fittest triggers) [16:38:33] <elexis> minus the question [16:39:10] <elexis> 4. in survival of the fittest you pass Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager).GetTemplate [16:40:09] <elexis> alternatively you could also delete the entire TemplateManager component [16:41:05] <elexis> also notice that still means nonvisual autostart needs a TemplateLoader, so the better approach is actually using if (Engine.GetTemplate) around that const template = Engine.GetTemplate("special/players/" + data.Code); block [16:41:06] <Freagarach> Well, it validates templates. [16:41:30] <elexis> Engine.GetTemplate doesnt? [16:41:38] <Freagarach> Nay. [16:42:01] <elexis> Gamesettings code doesnt use the civ data you inserted at all [16:42:11] <elexis> from what I see survival of the fittest doesnt either [16:42:26] <elexis> so these functions could just skip that Engine.GetTemplate call if Engine.GetTemplate isnt defined [16:42:44] <elexis> also it seems that this data should not used by simulation or gamesettings (nonvisual) in any case [16:43:20] <elexis> so thats a oneline JS fix instead of making a second template loader hidden inside a static local [16:43:51] <elexis> and he is going to introduce another local static template loader for the nonvisual autostart patch that is not needed [16:44:44] <elexis> so those are quick fixes, I dont know if its good to have that JS function combine the data of two files, or if it wouldnt be better to have it separated into nonvisual and visual data in two files. [16:45:04] <elexis> (and loaded by two different calls) [16:45:47] <elexis> (also dont know if it wouldnt be reasonable to just drop JSON at all instead of having XML here then sprinkling some JSON over there and switch back and forth all the time)
(This adds a proxy to cmpTemplateManager, it doesn't keep a static local template loader. But I do agree with the sentiment that globalscripts shouldn't have to rely on such details. Passing around a function not guaranteed to return the same output is also somewhat meh to be honest, cmpTemplateManager.GetTemplate shouldn't have to mirror TemplateLoader.GetTemplate for things to work.)
For the record -> This is intended as a temporary fix that nevertheless won't result in massive technical debt if it remains like that for the next 15 years, which I think is somewhat likely given that despite several tries, nobody's found a much better way yet.
The loadCivFiles debacle in particular just seems like it needs an engine function to list templates of some kind, as discussed with Freagarach on the diff. As for a global cache vs loading them separately, well that'll need to be done properly if it ends up being a performance concern.