[Hyperline] turning off/on no longer fixes dash trails.

[royal schooner]

using Everest 4163 core. my colors:

1dash: #9b3fb5
2dash: #f8c2f2```

trails are still vanilla. disabling and enabling the mod at least once from mod options no longer works as a workaround. recolors trails as intended if More Dasheline is disabled, but that's a SJ2023 dependency.

@summer lava is this fine?

[quiet osprey]

@neat cliff @rugged lake that looks like hook order isn't accurate. neither hyperline nor moredasheline use detourcontexts. both of them hook Player.GetTrailColor, which is the relevant method here. hyperline removes its hooks when disabled in settings and readds them when enabled (more dasheline's are only applied on startup as usual).
ideally one of these mods should use a DetourContext to make this not dependent on toggling settings like that, but this still signifies that hook order doesn't work as it used to

[neat cliff]

[rugged lake]

I'll look into it tomorrow its getting late for me

[rugged lake]

Still the same bug

I disabled DetourContext dispose and it works

prob some kinda jank hell with other mods that IL hooks somewhere that Hyperline hooks

Ill debug more after 💤

[neat cliff]

God damnit

@quiet osprey see above, not hook ordering but another instance of DetourContexts

It probably is a combination of it relying on implicit order of the hook load (which causes it to break when you toggle on/off), but outside of that a leaked detourcontext usually prevents this

[quiet osprey]

didn't you say leaked detourcontexts wouldn't happen because of some kind of scoping thing?

[neat cliff]

They leak within a method

Unless the scoping is also broken

[rugged lake]

Please no

That would be the worst

[neat cliff]

Ok it hopefully shouldn't be

[rugged lake]

Bad news it does

wait nvm it does by some magic voodoo this method leeks it context to Hyperline

Currently investigating why not Disposing LegacyDetourContext casues that

[rugged lake]

nvm it was a false positive, in core if you reintroduce the Disposed bug it leaks the context and somehow that fixed the issue but stable doesn't leak

[rugged lake]

Okay no idea how to fix this cuz Monomod , but the TLDR is that Hyperline isn't hooking correctly, it relays on the player unhooking and rehooking via the settings menu, this causes the hook to get placed in the correct place, but the issue with core is that LegacyHookEndpointManager.IsLegacyMMCaller return false because it cant find RelinkedMonoModLegacyAttribute in Hyperline EverestModuleAssemblyContext when the player rehooks the method from the settings menu.

@neat cliff

[rugged lake]

Okay after looking into the monomod stuff it seems that if a mod isn't using an assembly ref that contains MonoMod. it wont be considered legacy even if it uses MMHOOK_Celeste

https://github.com/search?q=repo%3Alordseanington%2FHyperline+Mono&type=code

Yap no mono.

[rugged lake]

My unprofessional fix https://github.com/EverestAPI/Everest/pull/662

[rugged lake]

btw found this while debugging https://github.com/swoolcock/GravityHelper/pull/82#issuecomment-1687699310

so GH hooks are not useless

[royal schooner]

me with my purple hair that just glows blue, watching how all this is blowing up

[rugged lake]

[quiet osprey]

does core not have MMHOOK_Celeste?

[rugged lake]

it does

core relinks all calls from HookEndpointManager in MMHOOK_Celsete to LegacyHookEndpointManager, the issue is that LegacyHookEndpointManager wont give a default config if the mod Assembly has no AssemblyReferences that stats with "MonoMod." .

[quiet osprey]

it seems like we need a better way to detect the version. do assemblies store the versions of assemblies they're linked against? we could bump the mmhook_celeste version on core

[rugged lake]

I'll look into it

[quiet osprey]

I checked if you can get the .net version an assembly is targeting, but it seems you can only get the version it was compiled on

[rugged lake]

not sure if dotpeek is anything to go by but it says that all mods are targeting MMHOOK_Celsete version 0.0.0.0

[quiet osprey]

I'm also not convinced having hook ordering differ between legacy and core mods is a good idea in the first place, that sounds like it'd make porting mods to core super annoying

[rugged lake]

that's a good point

[quiet osprey]

cc @neat cliff

since I assume the hope is that most mods won't use legacyRef forever

[rugged lake]

I mean that's good we could just search for version 0.0.0.0 and bump core to something else

[quiet osprey]

oh I assume IsNetCoreOnlyMod checks for the EverestCore dependency?

[neat cliff]

Yep

[quiet osprey]

I was hoping that'd be a thing of the past once core hits stable

[neat cliff]

Not really

Legacy mods will have to be supprted indefinitely

[quiet osprey]

anyway what are your thoughts on this

[neat cliff]

My thoughts are that the alternative is worse

The fundamental issue here is that legacy and reorg MonoMod mods both link against the same MMHOOK dll

However, the compat layer should only kick in for, well, compatibility reasons

And long term we want everyone to switch to reorg

[quiet osprey]

I mean "not having to change your dozens of hooks when moving your mod to core" is a compatibility reason

[neat cliff]

That's why it checks whether the caller of the MMHOOK method has been relinked against the monomod compat layer, which means that MMHOOK should also emulate the old hook ordering

[quiet osprey]

imagine when like extvar and gravityhelper start moving to core only mods

[neat cliff]

Core mods can still link against legacy monomod

That's orthagonal

[quiet osprey]

huh

[neat cliff]

The specific issue we encountered here is that if the mod links against no monomod version at all, we don't know whether it wants to use legacy or reorg monomod

Non-core only mods will always link against legacy, so we always use the fallback

However, for core-only mods we don't want to use the compat layer unless absolutely required

[quiet osprey]

that's why I suggested changing the mmhook_celeste version (assuming we can detect that). I don't think a mod that's not linked against monomod or mmhook_celeste can use hooks

[neat cliff]

If someone e.g. starts writing a new mod from scratch on core then they probably don't want the reorg layer

[quiet osprey]

actually no how would we handle the case of hyperline moving to core. it'd be linked against the new mmhook too

[neat cliff]

If someone moves to core-only however wants to force the compat layer (remember, this shouldn't be required at all because if you don't use MonoMod itself you as the modder don't seem to care at all about hook ordering), they can just add a reference to legacy MonoMod and not do anything more than that+

[quiet osprey]

only a handful of people intentionally care about hook ordering. but as we've seen repeatedly now, that doesn't mean they aren't relying on it implicitly

[neat cliff]

(fix pushed btw)

[quiet osprey]

this'll definitely require an expansive section in the porting guide

[neat cliff]

My idea is that someone entering the modding scene in 2024 will not have to care about anything fw or legacy mm related

[royal schooner]

what is bro hooking /j

[quiet osprey]

yeah no I'm talking about people porting their mods

[neat cliff]

If you start a completely new mod, and use MMHOOKs in a core-only setting, you definitely don't want to use a compatbility layer for some ancient out-of-date monomod version

[neat cliff]

They can either add a legacy monomod ref to force the compat layer (in case they implicitly relied on the old hook ordering), or they can just... not (in case they don't care, which should apply to 99% of people)

[quiet osprey]

also someone should make a pr to hyperline that (in a legacy compatible way) adds a detourcontext to that hook to make it take priority (since that seems to be the expected behavior)

[neat cliff]

Yep

[quiet osprey]

and hope that sean is still semi-active

[neat cliff]

As a quick overview, this only is an issue when:

• a non-core mod switches to core-only
• which doesn't reference any MonoMod dlls
• but adds an MMHOOK on a critical method
• in a way which only works with really specific hook ordering
• which is not explicitily specified using DetourContexts

In which case the fix is not to just make core restore the implicit hook ordering, but to just add a fricking DetourContext and make your intentions explicit like you are supposed to do

And if you keep your mod as a fw one (because you can't be bothered porting it), then you don't have to bother with anything

[neat cliff]

@royal schooner Core build 4171 is live now, which should fix the issue

[royal schooner]

the workaround is back :} thank you all

[neat cliff]

Marked as resolved

[summer lava]

i'm sorry, i been busy with work as of late

give me a moment to look through the whole thing

so, from what im seeing, this is an issue to swapping to a new system and causing the hooks to get out of order

what can i do to port/resolve this?

[rugged lake]

It issue was in Everest and was fixed, now Hyperline works fine on core

[quiet osprey]

the issue that made it behave differently on core has been fixed. however, ideally toggling it off and on to change hook order shouldn't be required at all, it should specify that its hooks should take precedence using detourcontexts. I'm not fully sure off the top of my head what the correct syntax for that would be because this stuff also has me thoroughly confused, but I can get back to you