Skip to content

ASC 3.0 Compatibilizations for HCMv2 #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: v2.x
Choose a base branch
from

Conversation

neimex23
Copy link
Contributor

@neimex23 neimex23 commented Mar 1, 2025

Compatibility Updates for HCMv2 to Work with ASC v3.0

Summary

This pull request introduces compatibility improvements to ensure HitCounterManager v2 (HCMv2) works seamlessly with ASC v3.0. The modifications address API changes, dependency updates, and ensure backward compatibility where necessary.

Changes Implemented

  • Updated API Calls: Adjusted HCMv2 function calls to align with the updated ASC v3.0 API.

Testing and Validation

  • Verified HCMv2 functionality with ASC v3.0 in multiple scenarios.
  • Conducted regression testing to ensure previous versions remain unaffected.
  • Performed stress tests to confirm stability under various workloads.

Notes

This PR is intended to maintain compatibility with ASC v2.x while fully supporting ASC v3.0.
Future updates may further refine integration based on feedback and additional testing.

References:
#36

topeterk added a commit that referenced this pull request Mar 7, 2025
@topeterk
Copy link
Owner

topeterk commented Mar 7, 2025

I checked in my proposal for the IAutoSplitterCoreInterface with a bunch of changes.
Let me know what you think about it, I try to explain each modification I made:

In general:

  • Reordered a bit to group profile related stuff or split related stuff, going from property before function and less detailed to more (e.g. first the main profile name property, then the profile list property, then profile related functions)
  • Renamed to a (in my personal view) more fitting name.
  • Some become a property as it is about plain data (allowing potential easier data bindings and makes two-way (read/write) access simpler/possible when applicable).

In detail:

  • Method ProfileName -> Property
  • Method GetProfiles -> Property ProfileNames
  • Method GetSplits -> Property SplitsNames
  • NewProfile -> ProfileNew (using same name as the methods that do this work in HCM already)
  • AddSplit -> SplitAppendNew (analogous to SplitInsertNew)
    Introduced a method for adding splits to the end of the list without the need to change active index at all while also setting the initial split name directly during creation.
  • ProfileHitGo -> HitSumUp
    Add method with same name internally to do a more convenient way than calling the increase or decrease operations for hits or way hits seperately.
    This also fixes the isse from the PR currently, as it does now take Amount (or with typo the "Aumount") into account rather than just calling HitInrease or HitWayIncrease once.
  • GetActiveGameIndexMethod -> Removed, please use already existing ActiveGameIndex property instead
  • ProfileChange and ProfileChangeTrigger -> Boiled down to ProfileSelectedMethod analogous to other methods that are populated by ASC.

I hope, you can agree with these changes or let me know, what you think about it?

@topeterk
Copy link
Owner

topeterk commented Mar 8, 2025

During testing of v2 I found some interessting things with AutoSplitterCore:

@neimex23
Copy link
Contributor Author

neimex23 commented Mar 8, 2025

1. Regarding the Trojan detection in SoulMemory.dll

It is most likely a false positive. The creator of SoulMemory faced a similar issue before: GitHub Issue. He may have resolved it for most antivirus software, but the issue might still persist for certain definitions. You could try excluding the file from scanning.

2. Issues with the AutoSplitter form

LiveSplit.Core and LiveSplit.ScriptableAutoSplit depend on libraries like System.CodeDom and System.Web, which only work on .NET Framework 4.8, not on .NET 7/8. Porting them would be a major challenge and could be counterproductive if we aim for compatibility with future LiveSplit versions.

The UI will open correctly, but when attempting to fetch the game list from SpeedrunComSharp, it will enter an infinite loop for the same reason.
When loading an ASL file, it throws exceptions due to the compatibility issue with:
CompilerResults compilerResults = cSharpCodeProvider.CompileAssemblyFromSource(compilerParameters, text2);
in ASLMethod inside LiveSplit.ScriptableAutoSplit. I attempted to migrate this to Roslyn, but without success.

3. Null pointer exception in CompositeGameList.cs

I changed the game to an existing one. The variable itself is not actually used, but it must be set for other LiveSplit functions. Changing it replaces the original error with a new exception in System.Web, which is caused by the same issue described in point

Here’s the new error:

HitCounterManager Error: 0 : Could not load file or assembly 'System.Web.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. The system cannot find the file specified.

   at SpeedrunComSharp.JSON.FromString(String value)
   at SpeedrunComSharp.JSON.FromStream(Stream stream)
   at SpeedrunComSharp.JSON.FromResponse(WebResponse response)
   at SpeedrunComSharp.JSON.FromUri(Uri uri, String userAgent, String accessToken, TimeSpan timeout)
   at SpeedrunComSharp.SpeedrunComClient.DoRequest(Uri uri)
   at SpeedrunComSharp.SpeedrunComClient.doPaginatedRequest[T](Uri uri, Func`2 parser)+MoveNext()
   at SpeedrunComSharp.CachedEnumerable`1.GetEnumerator()+MoveNext()
   at LiveSplit.Web.CompositeGameList.GetGames(Boolean loadFromCache)
Exception thrown: 'System.IO.FileNotFoundException' in System.Private.CoreLib.dll
Exception thrown: 'System.IO.FileNotFoundException' in SpeedrunComSharp.dll
HitCounterManager Error: 0 : Could not load file or assembly 'System.Web.Extensions, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. The system cannot find the file specified.

4. Changes in AutoSplitterMainModule.cs

Did you already test these changes? I might have overlooked updating that part.
Also, regarding SetActiveGameIndex, should I modify that as well?

@topeterk
Copy link
Owner

topeterk commented Mar 8, 2025

  1. I restored the file from quarantine .. was just curious because when the file gets deployed it might break installations. But hopefully they can solve it.
  2. Oh that's sad .. isn't this a major feature that is no longer working with HCMv2?
  3. Hmm, not sure why the files not loaded/missing. From my understanding the .Net framework should be supported (which should contain these files).
    [Edit] How did you changed to an existing game? The gameIDs is completely empty, I do not have a "speedruncom_game_names.json" file which should populate the list nor it is able to fetch the games from SpeedrunCom.Client.Games.GetGameHeaders() call.
    [Edit] The only issue seems to be the use of System.Web.Script.Serialization to do some JSON parsing. Is this stuff somehow important for ASC? If so, we could replace the JSON routines? Maybe as proposed here but seems to be a bunch of work?
  4. Haven't done any in-depth tests as I am not that familiar how to tell if it works in all circumstances. I just check if the reading/writing the settings via the interface works in general.
    As you do not get notified in ASC when the values gets changed in HCM we need some notification, the SetActiveGameIndex method is used for that right now.
    • (What seems to be "state of the art") We can implement INotifyPropertyChanged which means basically a method that gets called whan a value was changed by providing the respective object and the name of the property. Then you can implement a https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.propertychangedeventhandler and e.g. when the ActiveGameIndex gets changed by HCM, I can call the method and pass it a corresponding object (idk what object makes sense) and the string "ActiveGameIndex". The object is usually like a UI element (button) or a data object but therefore might requires deep knowledge of how HCM works to make actually use of that object. I mean, we can pass it anyway (just for easier debugging purposes or something) and just never use it in production code. Nevertheless, you would have to compare the given string and when it is "ActiveGameIndex", then you then need to read ActiveGameIndex and pass it to whatever ASC variable/object.
    • Alternatively, we can use a simplified version without passing an object, like public delegate void PropertyChangedEventHandler(string PropertyName); in case we have no benefit in providing an object at all. But would be non-standard.
    • Afaik, the first method is used by current frameworks, so if you plan to use data bindings in future versions, you might can bind the IAutoSplitterCoreInterface with your UI or data object. So, even when we do not have benefits from providing any object to the PropertyChangedEventHandler yet, it may pay out in future as we use a standard INotifyPropertyChanged interface.
    • [Edit] PS: This could be used to get rid of SetActiveGameIndexMethod, SetPracticeModeMethod and ProfileSelectedMethod.

@neimex23
Copy link
Contributor Author

neimex23 commented Mar 30, 2025

Updates about takings

Souls Memory is still getting flagged by Windows Defender. Someone already opened an issue about it: FrankvdStam/SoulSplitter#99. The maintainer mentioned it's mitigated, but not for Defender. Looks like Windows wants you to pay something extra to avoid this.

I'm currently working on a bridge to fix the ASL method in HCMv2. After a whole month of trial and error (and a bunch of headaches), I managed to isolate the logic into a new program. It communicates with ASC using Named Pipes and fixes the compatibility issues between .NET 7/8, web, and CodeDom. Honestly, a big achievement!

Check out this commit (everything works fine here): 8197819
Repo: https://github.com/neimex23/AutoSplitterCore/tree/master/ASLBridge
If you're curious how I solved it—maybe it's not the best approach, but it’s what I came up with.

As for the last point about implementing property change triggers (like INotifyPropertyChanged), I don't think it's necessary at this stage. I don’t see the project growing that much to make it worth it. The unit tests are minimal anyway, and as long as it works for one, it should work for all.

[PS] I'm doing some translation adjustments in the app, and probably tomorrow I’ll start tweaking the main page to prepare for the open beta. The site is built with React and hosted on GitHub Pages — check it out if you want: https://neimex23.github.io/AutoSplitterCore/

@neimex23
Copy link
Contributor Author

neimex23 commented May 6, 2025

Hey Peter, what’s up?

I’m having some trouble with the timer and maybe you can help.

A user reported that when the IGT timer is enabled, the HCM timer’s icon animation starts and stops—it’s not the values, they keep updating correctly, but the icon switches between active and inactive states. I’m worried this might be a bug that’s causing the internal timer to actually start and stop.

Interestingly, this doesn’t happen on HCMv1 with the current solution.
Here’s the code I’m using for the timers:
https://github.com/neimex23/AutoSplitterCore/blob/e1cc3aee5a891ecfeff4aed56e4d9858b800a811/Sources/ProgramModule/AutoSplitterMainModule.cs#L500

Remember IGT is take it by this function when HCM timer is started
https://github.com/neimex23/AutoSplitterCore/blob/e1cc3aee5a891ecfeff4aed56e4d9858b800a811/Sources/ProgramModule/AutoSplitterMainModule.cs#L285

@topeterk
Copy link
Owner

topeterk commented May 7, 2025

Hi, I'm fine thanks... regarding your issue, hmm..

The button updates the state in de UI depending on the TimerRunning property from the profileView data binding:

<ctrls:FramedImageButton MainColor="{DynamicResource ToggleTimerPauseBrush}"
OnClickCommand="{Binding #profileView.DataContext.ToggleTimerPause}"
Classes.TimerRunning="{Binding #profileView.DataContext.TimerRunning}">

The data binding is populated by the ProfileViewViewModel here:
<UserControl.DataContext>
<vm:ProfileViewViewModel />
</UserControl.DataContext>

So it all comes down to this logic:
private bool _TimerRunning = false;
public bool TimerRunning
{
get { return _TimerRunning; }
set
{
if (value == _TimerRunning) return;
if (!_TimerRunning)
{
// Starting the timer
last_elapsed_time = monotonic_timer.ElapsedMilliseconds;
_TimerRunning = true;
App.CurrentApp.StartApplicationTimer(TimerIDs.GameTime, 10, () => UpdateDuration());
App.CurrentApp.StartApplicationTimer(TimerIDs.GameTimeGui, 300, () => { CallPropertyChanged(nameof(StatsTime)); return _TimerRunning; });
}
else
{
// STOP THE COUNT!
UpdateDuration(true); // AutoSplitterCore might not always update the time, so we have to force update
_TimerRunning = false;
}
CallPropertyChanged();
OutputDataChangedHandler(this, new PropertyChangedEventArgs(nameof(TimerRunning)));
}
}

Which can be modified via the ASC interface by calling StartStopTimer:
public void StartStopTimer(bool Start) => ProfileViewViewModel.TimerRunning = Start;

In V1 the StartStopTimer is just simply enabling the one and only WinForms timer and in V2 this will spawn a new or delete the current timer.
Afaik in V1 when a timer stops, it will not fire any more that is why one might have to call UpdateDuration afterwards again manually. This is no longer needed in V2 as by setting the TimerRunning (see above) the UpdateDuration will be called automatically and then the internal running flag _TimerRunning gets reset. On the next timer tick the UpdateDuration method will return false and the timer will stop. When it gets started before the next tick the timer will be forcefully stopped, deleted, recreated and started, which happens all here:

public bool StartApplicationTimer(TimerIDs ID, double Interval, Func<bool> Callback)
{
StopApplicationTimer(ID);
DispatcherTimer timer = new (DispatcherPriority.MaxValue) { Interval = TimeSpan.FromMilliseconds(Interval) };
timer.Tick += (s, e) => { if (!Callback()) timer.Stop(); };
ApplicationTimers.Add(ID, timer);
timer.Start();
return true;
}
/// <summary>
/// Stops and removes the timer
/// </summary>
/// <param name="ID">The timer ID</param>
public void StopApplicationTimer(TimerIDs ID)
{
if (ApplicationTimers.Remove(ID, out var oldTimer))
{
oldTimer.Stop();
}
}

Therefore I do not expect that multiple timers are running in the background or anything might asynchonously modify the TimerRunning.
[Edit1] Just found out that V1 is also calling UpdateDurationInternal when a timer gets disabled, so when enabling or disabling the timer, it should never be necessary to call UpdateDuration manually, in both V1 and V2 versions.

But even when ASC is optimized for the new V2 timer behavior (basically just sparing some UpdateDuration calls), it should not break anything when it gets called multiple times. Same for TimerRunning as setting the value true/false when it is already true/false will just do nothing. But without any change in TimerRunning's value the icon should also not change. Therefore I have the feeling that multiple TimerRunning updates are happening that are disabling and enabling the timer just right after.
Could you have a look if StartStopTimer is somehow called multiple times and maybe check if its called with different paramter values? I could imagine that under WinForms settings different values multiple times betwee two render events may not be visible but due to the data binding event driven renderings it may somehow becomes visible?

Maybe as a test: Remove all StartStopTimer with false and only keep one with true to start the timer and then see if the issue is still there? Then we know that calls to StartStopTimer are not responsible for this issue, right?

So, I do not have a direct answer yet, but I will think myself through it again and I'll let you know when I noticed anything of importance.

@neimex23
Copy link
Contributor Author

Hey, quick update on this:

I simplified the logic in the functions, as seen here:
https://github.com/neimex23/AutoSplitterCore/blob/23b3eba8d06d7c6d832a024aaa919060c1decfe8/Sources/ProgramModule/AutoSplitterMainModule.cs#L492

Also, I’ve removed the UpdateDuration calls as we previously discussed.

The animation issue is now resolved.
As you pointed out, the real problem was that TryStopTimer and TryStartTimer were being called excessively—especially when using ASL scripts over the named pipe. The latency between requests was significant, and since I was checking for time differences to control the timer, the values were often identical, triggering rapid toggles.

I’ve adjusted the polling interval from 1000ms to 100ms in the named pipe client:
https://github.com/neimex23/AutoSplitterCore/blob/23b3eba8d06d7c6d832a024aaa919060c1decfe8/Sources/RemoteConnections/NamedPipeClientIGT.cs#L57

This helped reduce the delay noticeably. Setting it to 0ms would be too resource-intensive, so 100ms seems like a good balance for now.

@topeterk
Copy link
Owner

Hey, sounds great.

Not sure if this matter, so just for information: In HCMv2 the timer that is updating the label of the timer value is refreshing the text every 300ms - Also just an artificial number that just felt "right".

App.CurrentApp.StartApplicationTimer(TimerIDs.GameTimeGui, 300, () => { CallPropertyChanged(nameof(StatsTime)); return _TimerRunning; });

I think one have to see and maybe adjust it later a little bit when further feedback is collected. So, sounds like a good start to me.

If i am not mistaken, you already adjusted ASC to use the latest HCM code (ASC interface), right?
So, the only open changes of this pull-request are not need any more, correct?

Do we still have anything else, we need to look into?

@neimex23
Copy link
Contributor Author

The initial tests conducted over the past few months have yielded positive results on this end. I still need to continue bug fixing as the beta versions progress, but so far, no issues have emerged regarding the interface between HCM and ASC, for both HCMv1 and HCMv2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants