search

iseahound / ImagePut

A core library for images in AutoHotkey. Supports AutoHotkey v1 and v2.
https://www.autohotkey.com/boards/viewtopic.php?f=83&t=76633
MIT License
124 stars 26 forks source link

ImagePutSharedBuffer naming changes #50

Closed Descolada closed 3 months ago

Descolada commented 3 months ago

I propose some naming changes to the SharedBuffer option: 1) The FileMapping should hopefully be an unique enough name that there aren't naming conflicts with FileMappings created by other applications. For example the user might decide to use names such as "test" and accidentally get name conflicts. This PR proposes starting all FileMapping names with "ImagePut_" to increase uniqueness. 2) The default name should perhaps be "SharedBuffer" instead? Other ImagePut calls would be more readable, eg ImagePutWindow("SharedBuffer", "Worker Script Test #1") instead of ImagePutWindow("alice", "Worker Script Test #1").

iseahound commented 3 months ago
  1. What happens when a naming conflict is encountered? I was planning to push this problem onto the user.
  2. SharedBuffer might be nicer than default. Sentences like the default is default could be better worded. However, given the case-sensitivity I'd rather a single word all lowercase to reduce confusion with capital letters. So, shared or something similar?
  3. I like the idea of creating names with an ImagePut_ prefix but once again I don't like capital letters in a case-sensitive environment.
Descolada commented 3 months ago
  1. If you try to read from an already occupied FileMapping then you get garbage, but fortunately no invalid memory read/write errors. For example ImagePutWindow displays a 0-sized image if I create a file mapping with the content buf := Buffer(8, 0), NumPut("int", 4, "int", 4, buf). If you try to use ImagePutSharedBuffer on an already existing FileMapping you get the following error: image
  2. I would think a lower-case default value could run into the same problem if the user decides to try "Default" instead. The safest option would be to make the default value, eg "SharedBuffer", case insensitive by ImagePut. So in each OpenFileMapping call you could use "ImagePut_" (image = "SharedBuffer" ? "SharedBuffer" : image)
  3. The user doesn't know about the "ImagePut_" prefix as it would be an internal implementation detail. It would just be a way to create an unique namespace for your library in the context of FileMapping names. The user requests a SharedBuffer with the call ImagePutSharedBuffer("https://picsum.photos/800", "MyImage") and ImagePut internally converts it to ImagePut_MyImage. Case-sensitivity shouldn't be an issue here, although if you wanted you could make the ImagePut version case-insensitive by converting all the provided names to lower-case... Also, if the user is savvy enough to want to read the FileMapping directly, he can adjust his code accordingly to use the prefix.
iseahound commented 3 months ago

Thanks. I don't anticipate many people going the bare ImagePutSharedBuffer route without a name, so the default name shouldn't come up too much. Actually I highly doubt anyone will use this.