In D4530#197931, @vladislavbelov wrote:

In D4530#197910, @wraitii wrote:

God this code is a mess.

You're talking like other code is significantly better x)

God this code is a mess.

This one should work

Still learning lua

More debug

Debug

Fix premake

Fix cross-compilation, run with ARCH/HOSTTYPE=x86_64

Proper fix this time

Fix xwwidgets

In rP26881#57875, @Stan wrote:

Question how do I build a X86 build on my M1 now ? I can pass arch to the libs, but not to updateworkspaces.sh, nor premake for the game

Will be invalidated by D4646, which seems easier given the rest of code changes, and since this is old/stale I'll abandon directly.

For Demo purposes, add a RunThreaded function to run some threaded javascript. This highlights that a web-worker like API is doable, but doing it efficiently would take a little more thought.

Rework, upgrade & make actually functional.

In D4575#197726, @wraitii wrote:

• If used in the GUI, we'll need to do some work to expose the init function, unless we have a non-module file to describe page entry points. Something like import { init } from './HotkeysPage.mjs'; globalThis.init = init; works, but it's kind of ugly.

Tested this quickly today:

• The '.mjs' syntax might be useful to differentiate in the engine. So maybe let's keep that.
• If used in the GUI, we'll need to do some work to expose the init function, unless we have a non-module file to describe page entry points. Something like import { init } from './HotkeysPage.mjs'; globalThis.init = init; works, but it's kind of ugly.

Fix last comments.

Change #elifdef to #elif defined()

This is included in D4607 & seems to bug for other people as well (see phosit on IRC today). I think it might be a Python 3.10 thing instead of a mac thing.

Reworked using an upstream patch of mine (but largely the same fixes). Tested to work on my mac.

Raise error, handle errors in the C++ (fail to autostart)

In D4575#197572, @wraitii wrote:

Do you have something specific in mind as to not really interchangeable?? I think they generally are.

In D4575#197559, @smiley wrote:

Either /, ./ or ../ at the start I guess. None of those would mean start at the current directory.

In general I'd rather not have a leading / unless it's necessary for disambiguating. In frontend development, absolute paths usually refer to node_modules and the root source directory is aliased to something (I think @ is most common).
However, we don't have node_modules and likely never will, and C++ vfsPath just start at the VFS root in general. I think it makes sense to have three types of paths:

• Absolute paths (from VFS root), no specific syntax.
• Paths that start with ./, which are relative paths.
• Paths that start with ../ any number of time, which are also relative paths.

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.

In D4575#197178, @wraitii wrote:

In D4575#195007, @smiley wrote:

Notice that it won't be possible to resolve out of VFS root.

Definitely the only sane move.

Read-through review:

• I think we should use .js over .mjs in the interest of tooling support, and I'm not convinced that .mjs will actually take off.
• There's a bunch of duplication with LoadGlobalScript & such in ScriptInterface.cpp. I think it's time to merge those together in a single ScriptCompile file or something.
  • In particular, there's a question of entry points -> Once we're compiling as modules, we can import submodules, but we can't import modules from non-modules bits.
• Your tests are missing an absolute path, and I think it's broken in the current implementation. I think it should work, and should start from the VFS root as one would expect. Relative paths are quickly unwieldy when you get into folders-of-folders.

Always load the network JSI functions.

Think this approach is workable, will let freagarach take over :)

In D4643#197432, @Mercury wrote:

So If I add changeOrigin to Reduce and to RegisterHealthChanged then I could add it to the HealthChanged message.
Does this sound good? If so should I pass attackerOwner along with it?

Hm, hadn't though of loot. Your right, that's more annoying.

I think we found a better idea, see comments in the fixing diff that ended up being committed.

As for some knowledge drop:

In D4643#197406, @Mercury wrote:

SendGlobalMessage(ent, msg);
that doesn't seem right? The messages are sent to the components they are supposed to go to on line 990?

In D4643#197400, @Mercury wrote:

Looking at source/simulation2/system/ComponentManager.cpp line 974 it seems to be saying that when you provide an entity to PostMessage only the components of that entity will be notified.

C++ changes only, CC

In D4640#197295, @vladislavbelov wrote:

In D4640#197284, @wraitii wrote:

You might run in a situation where clipart == CRect(), but I'm not sure that's actually something that can happen in practice.

Currently it's intended to not clip if clipart == CRect().

For reference, AoE 2 added a ranged/melee unit recently: https://www.youtube.com/watch?v=rvJdOKktoMA
The switching is not automatic but manual.

In D4640#197283, @vladislavbelov wrote:

In D4640#197280, @wraitii wrote:

not clipping if the clipping area is 0 by 0

Why?

As I pointed out in the forum, 0 and undefined aren't actually equivalent anyways. The formula we use is:
effectData.Damage[type] * Math.pow(0.9, resistanceStrengths.Damage[type]). You'll notice that Math.pow(0.9, 0) is 1, whereas Math.pow(0.9, undefined) is NaN, as one would expect. Thus a guard must be added, and thus there is an obvious difference.

High-level comments:

• I think this is something we should move towards. It's saner than our current code in the GUI IMO, and could lower load times and sort out the common mess somewhat efficiently.
• I think you'll need to have the module cache at the ScriptInterface level but I would need to check the SM docs
• We have a question for loading whole directories, but maybe we should treat it as an implicit import * as FILENAME from FILENAME.js
• We need to consider how to handle hot reloading as well.

In D4461#190337, @Langbart wrote:

Its better, but they still switch unitAI gather/approching state.
EDIT1: better video, which compares pushing with rP26244 (left) vs rP26261 (right)
[ Video ]

Commonalize code, maybe fix serialisation issue, am confused by why it seemed to work before.

In D4629#197063, @Freagarach wrote:

You'll need to update cmpUnitMotionFlying to also update the clearance in cmpObstruction.

remove Civilisations.js, failed to notice that.

• Move gamesettings from gui/gamesettings/ to gamesettings/. This moves attributes as well.
• Move autostart files from gui/autostart/ to autostart/. Note that gui/autostart/ still exists as it's used in the tutorial button from the main menu.
• Implement a placeholder autostart/entrypoint.js in the mod mod.
• Rename the JSI_VFS functions to be explicitly about their function: the ability to write and the potential restricted folders.
• Load the full autostart/ folder instead of just the entrypoint script to allow easier extensions in mods.
• Don't require autostart when using --autostart-client in non-visual mode.

In D4461#197143, @marder wrote:

To me it seems like this is just a bad edge case since the house is so close? I didn't notice that it would happen extensively, but maybe I'm wrong.

spaces

In D4486#197132, @Freagarach wrote:

In D4486#197129, @Stan wrote:

So do you and @wraitii agree on this or are you still debating which of the patches is the right move ?

Good question. :) I haven't seen a formal agreement on the patch level, but our discussion on Friday seemed to hint to this patch.

I think we've largely agreed that long-term, the proper fix here is to implement a templateLoader-level ListTemplates(TemplateType) function, and use different root elements (<Entity>, <Technology>, <Civilization>).

In D4632#196919, @Freagarach wrote:

Furthermore, basically all callsites have been complicated by the diff instead of simplified, which shows IMO that something is awry.

I see only the call in templates.js being complicated?

In D4628#196888, @vladislavbelov wrote:

Yes and the patch adds the code somehow depending on GUI:

// ...
scriptInterface.LoadGlobalScriptFile(VfsPath(L"gui/autostart/autostart_entrypoint.js"));
// ...
JSI_VFS::RegisterScriptFunctions_GUI(rq);

In D4628#196779, @vladislavbelov wrote:

So the patch basically adds an easier possibility to break the non-visual mode in the future (as the new code should be always synced with the logic of GUI).

Gonna raise a concern, I've convinced myself this needs to be mostly reverted. See https://code.wildfiregames.com/D4630#196827, rephrased below:

I've considered smiley's comments that this should just take a template. But trying to do that was super awkward. This had got me thinking that rP26298 was essentially the wrong move.

Might be worth abandoning the diff then, but I seem to generally agree.