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

ByRef Broken with v2 Beta 3 #31

Open
brubsby opened this issue Mar 2, 2022 · 7 comments
Open

ByRef Broken with v2 Beta 3 #31

brubsby opened this issue Mar 2, 2022 · 7 comments

Comments

@brubsby
Copy link

brubsby commented Mar 2, 2022

Hi, the version semantics for ahk v2 beta are confusing to me, and it's unclear what version of the v2 beta I have, so I'm not sure if I'm ahead or behind the supported ahk version of this repo (I just downloaded the most recent release of the beta from the ahk website labed v2 beta 3)

But it seems the byrefs in this library are now unsupported in v2:
https://lexikos.github.io/v2/docs/v2-changes.htm#byref

which causes this error:

Error at line 396

Line Text: CreateRectF(ByRef RectF, x, y, w, h)
Error: Missing comma

The program will now exit.

Unsure of your plans for this repo and if you had achieved your goal originally. But I was hoping to migrate my scripts that depend on Gdip to v2, so that I could use other v2 libraries that are incompatible with v1. I'm not opposed to downgrading my v2 version in the meantime, but I'm not sure how to get my hands on build a108. The link your readme points toward the version I currently have.

@mmikeww
Copy link
Owner

mmikeww commented Mar 3, 2022

Yeah, this repo is not up to date with the latest AHK v2 updates. Usually the README is correct, I haven't checked in a while, but I'd expect that if it says a108 then that is what it would work with.

As far as the semantics, the aXXX versions such as a108 were for the "alpha" builds. Those were the early builds where many breaking changes were introduced. Then after that came the "beta" builds, which are designed to be more stable with less breaking changes going forward. And then I guess eventually they will release the stable version after that.

It looks like you can download the older v2 versions here:
https://www.autohotkey.com/download/2.0/
But if you downgrade to a108, then maybe your other v2 code might not work, if your other code is designed for a certain newer beta version.

Unfortunately there are no good solutions for you, until I or someone else updates this lib for beta3.

You could take a shot at making the changes, and then contribute by submitting a pull request here yourself, if you wanted. For example, that ByRef change looks to be fairly simple to update, along with updating all the calls to the func as well with the &var prefix

@brubsby
Copy link
Author

brubsby commented Mar 3, 2022

Thanks for the response! I'll definitely consider giving it a shot if you don't have plans to any time soon, although I'm unsure how/if the backwards compatibility would be able to be maintained with so many breaking changes. And I'm sure there's more hidden ones as well since it's so many versions apart.

@mmikeww
Copy link
Owner

mmikeww commented Mar 3, 2022

Yeah feel free to attempt it. I had no immediate plans to make updates. But yes of course, I completely forgot that one of the goals of this lib is to be backwards compatible with AHK v1.1

I'm not sure if these ByRef changes would work in that case since the interpreter would likely throw syntax errors on loadtime like in your screenshot.

For this specific example, I just searched the file and it looks like CreateRectF is only called 3 times throughout the lib. The function is fairly simple, just creating a struct. It seems easy enough to just modify the function to return the struct, instead of doing the implicit return via a ByRef. So for this change, we could just remove the ByRef param, and return the struct, or if AHK emptys the memory and doesn't allow the struct to be returned, then we could just eliminate this func entirely, and just inline the 2 lines of code in the 3 places where the func is called.

That is a simple change for this specific CreateRectF func. However, I see there are quite a lot of ByRef parameters in many other functions, and those changes will not be as easy. In most cases, using a ByRef parameter is simply a means of returning multiple values out of a function. This was useful back in the AHK Basic days when you could only return 1 value. But since we have objects, we can always just change the function definition to return an obj instead. So for example:

; return				No return value

Gdip_GetImageDimensions(pBitmap, ByRef Width, ByRef Height)
{
	Width := 0
	Height := 0
	Ptr := A_PtrSize ? "UPtr" : "UInt"
	DllCall("gdiplus\GdipGetImageWidth", Ptr, pBitmap, "uint*", Width)
	DllCall("gdiplus\GdipGetImageHeight", Ptr, pBitmap, "uint*", Height)
}

could be changed to:

; return				Object with two keys "w" and "h"

Gdip_GetImageDimensions(pBitmap)
{
	Width := 0
	Height := 0
	Ptr := A_PtrSize ? "UPtr" : "UInt"
	DllCall("gdiplus\GdipGetImageWidth", Ptr, pBitmap, "uint*", Width)
	DllCall("gdiplus\GdipGetImageHeight", Ptr, pBitmap, "uint*", Height)
	return {w:Width, h:Height}
}

Something like that, where we update the comment of the function too. And then when the function is called, the call would have to be replaced from:

	Gdip_GetImageDimensions(pBitmap, Width, Height)

to

	dims := Gdip_GetImageDimensions(pBitmap)
	Width := dims.w
	Height := dims.h

You get the idea

@dmcnaugh15
Copy link

I'm not feeling brave today. Are you aware of any unofficial updates out there?

@dmcnaugh15
Copy link

Found this one: https://github.com/buliasz/AHKv2-Gdip

@maessof91
Copy link

Doing a regex search replace seemed to solve this part for me

Search: "ByRef \b"
Replace: "&"

worked for men

@mmikeww
Copy link
Owner

mmikeww commented Jul 21, 2024

Doing a regex search replace seemed to solve this part for me

Search: "ByRef \b" Replace: "&"

worked for men

yes that might work for v2, but this library is supposed to be backward compatible so that it works for both v1 and v2. however, if that is no longer possible, maybe we just drop that attempt that this point. i'm not sure

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

No branches or pull requests

4 participants