Skip to content
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

The v0.3.7 release is messed up #29

Open
bluechipps opened this issue Feb 18, 2019 · 14 comments
Open

The v0.3.7 release is messed up #29

bluechipps opened this issue Feb 18, 2019 · 14 comments
Labels
bug Something isn't working

Comments

@bluechipps
Copy link
Contributor

bluechipps commented Feb 18, 2019

  1. If running x64 version of AHK it will fail.
    "You should extract both x86 and x64 folders from the library folder in interception.zip into AHI's lib folder."
    There is no x64 folder in the zip file, only an x86 one so either need to change how interception searches for dlls or the folder needs to be created (and a 64bit dll placed in it).

  2. There is an "interception.dll" in the base Lib folder inside the zip, unfortunately it also is x86. So there are no x64 dlls included in the current release,

@archit9169
Copy link

archit9169 commented Feb 22, 2019

  1. If running x64 version of AHK it will fail.
    "You should extract both x86 and x64 folders from the library folder in interception.zip into AHI's lib folder."
    There is no x64 folder in the zip file, only an x86 one so either need to change how interception searches for dlls or the folder needs to be created (and a 64bit dll placed in it).
  2. There is an "interception.dll" in the base Lib folder inside the zip, unfortunately it also is x86. So there are no x64 dlls included in the current release,

Download interception.zip from the link. Read the instructions. Copy the x64 and x86 folder from that zip and paste it inside lib folder of ahi. I am using it and it works.

@archit9169
Copy link

archit9169 commented Feb 22, 2019

@evilC you might want to delete the default interception.dll from the lib folder and instead supply both x64 and x86 folders from interception.

Please close the issue.

@evilC evilC closed this as completed in c6daabf Feb 24, 2019
@evilC
Copy link
Owner

evilC commented Feb 24, 2019

I supply the x86 version, because many people, even on x64 systems, just use x86 AHK, because then compiled scripts etc work on all PCs.
The instructions, however, are not accurate, I have updated them.
Thanks for the heads up

@nikolovjovan
Copy link
Contributor

If you recall my pull request #22 I have changed the way AutoHotInterception library loads interception.dll by loading the correct bitness automatically from the x86 and x64 folders inside the Lib folder. Seems like you forgot how it works and added Lib\interception.dll and Lib\x86\interception.dll to v0.3.7 which is both unnecessary and I think violates Interception license (LGPLv3) since you forgot to include the license with the release zip. You also "fixed" (broken) the build event with 9c80a5b by removing my script and restoring yours instead of actually fixing your own workspace like I commented on that exact commit. Alas, I would like to help you sort these little issues out. I have been working on my own fork for quite some time now but I believe it has changed to much to issue a pull request. I would appreciate if you took a look at it and if you want we could work on implementing those changes on this repo.

TLDR: ReadME needs a build guide to fix these simple problems and clearer instructions...

@evilC
Copy link
Owner

evilC commented Feb 25, 2019

I made the mess, I guess I need to clean it up...
Thank you so much for persisting to remind me on this one, I will try and get it done ASAP. It may not be tonight as I have a social engagement, but hopefully tomorrow if not.
Regarding merging in your changes, having a quick scan through now.

I don't see too much in the way of new functionality, only really the concurrency switch, otherwise it just seems to be tweaks to the implementation? There also seems to be a shit-ton of formatting changes (Spaces, line breaks etc) which I guess will make it harder to merge.

Do you maybe want to hook up at some point in Discord or something and we can go thru what you have changed? Either I can merge my changes into your branch, or vice versa, depending on what we decide would be best.

@evilC evilC reopened this Feb 25, 2019
@nikolovjovan
Copy link
Contributor

Yeah I would like to integrate those changes into the main repo but as ReSharper got mad and I hate it when it spews yellow warnings I let it do a code cleanup. Well the most significant change as you have said is the concurrency switch with it being off by default meaning the events will be processed in a single thread per subscription opposed to it being multi-threaded with a thread pool. Of course the TestApp is tuned for my specific hardware and it is only testing the concurrency. My problem was when I wanted to use the mouse wheel as a volume controller with it being jittery and jumpy. So after many hours of trying to fix it in AutoHotkey I decided to change the library itself...

@evilC
Copy link
Owner

evilC commented Feb 25, 2019

I don't see why mouse wheel would be inherently jittery for volume control?
Maybe if you were using a mapping such as WheelUp::Volume_Up, because WheelUp has a press but no release event, so maybe you would end up pressing Volume_Up but never releasing it?
In that case, I guess WheelUp::Send {Volume_Up} would probably fix it.
If that is intermittent, then most likely the reason is that you are not holding Volume_Up for long enough. Many multimedia apps process input in a similar way to games, and as such, typically need keys held for 20-50ms. In that instance, adding SetKeyDelay, 0, 50 to the start of the script should fix it.

@evilC
Copy link
Owner

evilC commented Feb 26, 2019

OK, so I just sat down to start fixing this mess, but it looks like you beat me to it?
Looking at your repo, it looks like you merged in my latest changes.
I merged in changes from your master branch into my crumbl3d branch. and changed the docs, made stub folders and readmes etc, hopefully everything should be really obvious (And legally compliant!) now.
If you could give it a once-over to make sure I have not missed anything, that would be great, then I can push out a new release.

@evilC
Copy link
Owner

evilC commented Feb 26, 2019

Oh yeah, and notice my tweaks to the syntax of your build events. $(TargetDir) ends with a \ anyway, so adding one after just results in \\, and also I add /Q /Y switches to the Xcopys, else it seems sometimes builds fail and you have to re-build (even if you use if exist etc it seems) - on my system at least.

@nikolovjovan
Copy link
Contributor

I don't see why mouse wheel would be inherently jittery for volume control?

I didn't really use the media keys to change volume but implemented my own little class for adjusting volume using SoundSet and SoundGet... If you are interested you can take a look here. Since each event is processed in parallel and every time it is handled I get current volume, change it, set modified volume and they weren't synchronized it was failing miserably. That's why I implemented the single-threaded event processing...

If you could give it a once-over to make sure I have not missed anything, that would be great, then I can push out a new release.

Sure, I'll post a reply here when I'm done!

@nikolovjovan
Copy link
Contributor

nikolovjovan commented Feb 26, 2019

Okay so it is building successfully once you add the interception dlls.

On the side note I think you kinda went overboard with the readmes and that there should be only one build instruction file in the root of the repo. That could be done later on though.
What you could do right now is just move the main Readme.md from the Dependencies folder to C# folder (one folder up), change it accordingly and simply remove everything from the Dependencies folder (remove the x86 and x64 folders). So the readme would look something like this:

In order to build, the Interception DLLs need to be placed in **Dependencies** folder.  
It should contain `x86` and `x64` folders, as extracted from the `library` folder in the interception zip.
YOU MAY ALSO NEED TO RUN UNBLOCKER.PS1 AS ADMIN!!

@evilC
Copy link
Owner

evilC commented Feb 27, 2019

One of the points of having it in there was to force the Dependencies folder to exist in the repo.
I don't know how to make the folder exist without there being something in there.
Oh, and BTW, I started looking at your implementation for the concurrency - this producer-consumer pattern seems to be more in line with what I want rather than just doing every callback on a new thread, I just never got around to learning how to do it right.
I am not sure, therefore, I really see the point of having both methods and a switch, or at the very least I think it should default to your mode - the average consumer of this library is probably not wanting to deal with multiple threads.

@thorerik
Copy link

thorerik commented Mar 3, 2019

@evilC typically the way to do that is to commit a file named eg. .gitkeep into the otherwise empty folder

@evilC evilC added the bug Something isn't working label May 14, 2019
@evilC
Copy link
Owner

evilC commented May 14, 2019

Should be fixed in 0.4.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants