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

Add -path option for injector (#2) #6

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rougemeilland
Copy link

This is a proposed fix for issue #2.

By specifying -path <initial directory path name> as a command argument for Dll_Injector.exe, you can change the initial value of the directory where images are saved.
The -path command argument is optional.

By the way, the code page of the original common.h was not UNICODE, and it contained comments in simplified Chinese.
When I edited common.h, there was a possibility of garbled characters, so I changed the code page to UTF8 with BOM.
note that.

@JamesHoi
Copy link
Owner

JamesHoi commented Oct 14, 2023

A pull request should be focused to one issue. I think the chinese comment should be removed or change to english but not in this PR.

@JamesHoi JamesHoi changed the title Fixed issue #2 Add -path option for injector Oct 14, 2023
@JamesHoi JamesHoi changed the title Add -path option for injector Add -path option for injector (#2) Oct 14, 2023
@JamesHoi JamesHoi marked this pull request as draft October 15, 2023 06:16
@rougemeilland
Copy link
Author

The code page of common.h before editing was EUC (Simplified Chinese), and in my development environment I could not edit it without causing garbled characters.

Therefore, I changed the code page to UTF-8 and translated the comments to English.

please confirm.

@rougemeilland rougemeilland marked this pull request as ready for review October 15, 2023 12:24
const char* initialPath = "";
for (int index = 1; index < argc; ++index)
{
if (strcmp(argv[index], "-path") == 0 && index + 1 < argc)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single dash usually use in the short form of argument option, I think it is better to use two dash like --path.

@JamesHoi
Copy link
Owner

JamesHoi commented Oct 15, 2023

Also, this dumper is not for CLI user and thats why I use gui to choose save directory. I am not quite sure this PR is the best way to let user choose the initial save directory.

For example, if I am a CLI user, why do I just use the argument like --path as the saving directory. On the other hand, if I am a user who doesnt know about CLI, why this software cannot automatically choose the last saving directory as the initial path. In my opinion, the argument of initial path is totally useless.

…f the directory where image files are saved has been changed from "-path" to "--path".
@rougemeilland
Copy link
Author

As you pointed out, I changed the command argument from "-path" to "--path".

I would also like to answer why I am particular about specifying command arguments.

I also have no intention of using Dll_Injector.exe with the CLI.
The reason why you want to specify it as a command argument is because you are considering running it from a shortcut.

When you run Dll_Injector.exe from Explorer or from a shortcut pasted on the desktop, the list under the desktop is always displayed as the destination for saving images.

As mentioned above, I wanted to save images in a dedicated directory, and having to reselect the directory every time I started Dll_Injector.exe was very tedious.

If you can specify the initial directory where the image will be saved using the Dll_Injector.exe command argument, you can save the above effort by specifying the command argument along with the full path name of the executable file in the shortcut properties.

As another workaround, I considered a method of ``displaying the last selected directory first at next startup,'' but I ultimately gave up on this method due to the following requirements. .

  • Concerns about the amount of modification for the code that saves the “last selected directory path name”
  • Where to save the "last selected directory path name".

@JamesHoi
Copy link
Owner

JamesHoi commented Oct 15, 2023

I have totally understand the situation but I dont think implementing of saving software settings need a lot of modifications. I think use windows regedit for saving and loading settings just need little changes.

PS: You should always deal with one problem in one PR, we should solve the common.h file encoding issue in another PR but still thanks you for improving this repo.

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