topeterk / HitCounterManager

Free Hit Counter / Death Counter that is running in the background, so you can focus on your stream. No need to keep any windows open for a window capture any more. Initially designed for Dark Souls and similar games but supports any game.
MIT License
86 stars 20 forks source link

Added AutoSplitterExtension Support #31

Closed neimex23 closed 2 years ago

neimex23 commented 2 years ago

Hi, I send a new pull request with only modifications on HCM of Split Of Pleasure Update. With release of LiveSplit.HitCounterManagerConnector, AutoSplitterCore is complettly obsolete, With the excuse of having the possibility of autosplitting in hcm without having livesplit open, the maintenance of the extension continued, i think that the best way is send hcm with support to AutosplitterCore, so if pepole want to install AutoSplitterCore go to my github and should only download and install extension, It gives me the possibility of having more flexibility when updating the extension.

topeterk commented 2 years ago

Hi, yes, as the DLL is "optional", I think this is a good idea releasing HCM and your DLL separately. It makes it easier releasing a new DLL without having to also release HCM as well (which might not have changed at all). 👍

topeterk commented 2 years ago

Please have a look at the merge/autosplitterextension branch. I tried to get through everything and did a bunch of reworking.

If this is fine so far I can merge this part.

However before I will do a release I would like to get rid of the hard coded game name list of the combobox. May you want to provide a function that HCM can request the game list from the AutoSplitter DLL, then you should be able to add more games in the future without modification of HCM.

And just 2 little findings: 1) (Just personal feelings) The timer update will produce many writes on the output due to the "force" flag, not sure if - instead of counting amount of writes - it is possible to do updates only when needed (e.g. wenn timer stops or values do not match with the already stored ones or something like that). As already mentions too many writes on the file MAY have impact on the stability of the rendered HTML file when the file is read multiple times exactly at that time when the file is rewritten by the HCM. Reducing the writes is recommended when possible, just want to mention it. 2) Maybe you don't feel like this is any real world use case but when the AutoSplitter DLL gets removed and hotkeys for AutoSplitter were enabled in the settings, then the configured AutoSplitter hotkeys cannot be removed or disabled any more by the HCM (only via editing the XML file). One have to either prevent loading the hotkeys when the DLL is not being loaded or in case the DLL is not found, the hotkeys will be set disabled, so on the next startup they will no longer be loaded (in case this issue should be solved at all).

neimex23 commented 2 years ago

Im fix the problem with hotkeys now is dll is attached this dont load in the system and, in the next save (if user save) hotkeys values turn on default values.

I do a new function that return a list with games in the system and this build the combobox items.

About point 1 is big problam that i have when igt values track was made, im talking with peoples that dev those function if can try found any fix, but at moment nothing, im I'll keep you up to date on this but I don't see a future for it. Work yes, work fine yes, igt and timer are same yes, works efficient no :(

topeterk commented 2 years ago

Thanks for improving the two mentioned points. I have integrated the change into merge/autosplitterextension branch.

For the open one: We just stick with the current implementation and if there can be improved anything, we can have a look at it later again.

Do you have anything that shall be part of the next HCM version or do you think this is all for now? Then I would merge it and plan to make a new release in the near future (I still have to check, if I have anything on my list before the next release, so it might be delayed for a bit longer if this is the case).

neimex23 commented 2 years ago

Is done for now im added Load PracticeMode from my Xml Settings that i forgett and i add a new instrucction on SetDuration Function (duration - GetSplitDuration(LastActiveSplit)) >= 1000 (only if this condition sussess SetDuration in the split). i dont know if this fix the problem of html but i think that decrase count of writing. is a starter Thx for All and support for this collaboration :)

topeterk commented 2 years ago

Hi, the condition will just not update the internal values, the output is not affected by it. I have integrated the last changes into merge/autosplitterextension again and reworked the function a little bit. Do you think this may work, then I will keep it this way or shall we change it again?

neimex23 commented 2 years ago

Oh i see, no i don't add anymore thx

neimex23 commented 2 years ago

Hey im should release a new version of autosplitter and i take the opportunity to use hcm with merge/autosplitterextension because "more clearly code". I realized that an instruction in form for PracticeCheck is wrong your put a type instead of instance, i change return of igt values to long to help you with SetDurationByCurrentTotalTime Function, for me I keep the old code because the current generates StackOverflow Exception i commented you code for dont lost it.

topeterk commented 2 years ago

Do you know how the callstack looks like when you get the stack overflow? I don't think we are at such a limit that a single long value will crash within that function? Is this happening only when it produces output? What thread is running into this issue? Does it also happen in release build?

And with your "fix" we get old issues again (outputs without anything changed or updated internal values but no output update).

neimex23 commented 2 years ago

im checking again and i found this, the error is caused by this instruction:

if ((ForceUpdate) || (Math.Abs(duration - GetSplitDuration(ActiveSplit)) <= 1000)) if you change < (smaller than ) for > (bigger than) this fix the problem i think that is the correct way "i want set a duration if the diferent between duration and currentSplit is bigger than a 1 sec

whatever i send you how look stack in a debug mode: [image: imagen.png] i think that is main thread and is after get values of igt function, i put the value in a separated var [image: imagen.png] and you see error happen in the next intruction. And happen in debug mode and release version

"My Fix" is a temporal fix nothing else I used it to release a new version, we are still working on it :)

El mié, 5 oct 2022 a la(s) 17:35, Peter Kirmeier @.***) escribió:

Do you know how the callstack looks like when you get the stack overflow? I don't think we are at such a limit that a single long value will crash within that function? Is this happening only when it produces output? What thread is running into this issue? Does it also happen in release build?

And with your "fix" we get old issues again (outputs without anything changed or updated internal values but no output update).

— Reply to this email directly, view it on GitHub https://github.com/topeterk/HitCounterManager/pull/31#issuecomment-1268945561, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHUGLDLGQT3VZY2XXTROFVTWBXRCHANCNFSM6AAAAAAQPTPD4A . You are receiving this because you authored the thread.Message ID: @.***>

topeterk commented 2 years ago

Maybe the issue is when output is generated, it calls profCtrl.UpdateDuration(); which will again update the timer values that may again produce output which updates again? But if this is an issue, we most likely would see recursion in the callstack of these functions? This was no issue without AutoSplitter as duration updates have not fired any events triggering new output before.

PS: The images you put into the last message are not visible, unfortunately.

neimex23 commented 2 years ago

Oh nice github thx for interpret the images Stack devenv_BlBM8WhZwT

And Instrucction cause error after get igt values SGwigHI0Qr

neimex23 commented 2 years ago

oh i see the recursion 1080 times

topeterk commented 2 years ago

Hmm, maybe remove the update call from OutModule when AutoSplitter is active? At least as a quick fix...

neimex23 commented 2 years ago

i have a thing that i dont understend by the functions (Math.Abs(duration - GetSplitDuration(ActiveSplit)) <= 1000

why i want set a duration if diference is smaller than 1sec and not if bigger than 1sec?

btw i try added a if in a outmoudle

AutoSplitterLoaded is seted on true when SetIGTSource in ProfilesControl happen. Dont fix stuck overflow if funtion is < = 1000 but if > = 1000, run but dont see any changes. How can i view counting of writes in html if what I do has an effect or not

topeterk commented 2 years ago

I had a second look at this today and I think I found a quite easy protection for this issue. https://github.com/topeterk/HitCounterManager/commit/075d034952cb878492fd06b0a5ac470736452df3 It should also work in other cases besides AutoSplitter or timer updates in general (even when there are none of them yet). May you check that again if it also works for you?

The condition was a typo. Good that you catched that :)

To get a feeling when updates are running I placed a Console.Beep() at the Update() function (is just commented out) but with the latest code you may look for the new IsUpdateRunning = true; in case you don't want to hear false positives.

neimex23 commented 2 years ago

I did it, i made that this work finally, the beep help me a lot now update() in outmoudle is produce 2-3 times only if a game is loading, after this no more. From beeps betwen half second to beeps only when game is loading, im should change any thing for my part in the dll but now work perfectly, I took the opportunity to clearing my code, order and optimizing, i remove some functions of MethodInfo in form.cs that was useless there and in dll.

neimex23 commented 2 years ago

ProfileUpdateBegin(); ProfileUpdateEnd(); were problems i dont know how work without this functions before dont changing html, now yes i dont know im tested a lot of time and i dont see any problem at moment

topeterk commented 2 years ago

Output should be generated when the split selection changes or when timer starts/stops, so I guess the events should always trigger the output generation at that time. Sure, we haven't missed anything? I'm just curious as you put in writing the output on timer updates in the first place. If this is fine, great that we solved it.

neimex23 commented 2 years ago

Tested on all games availables working perfect :) only beeps on timer start or stop, i dont know the first release of this was made by other peoples, nowday they vanished and was left with the doubt of how they thought it