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

AddVariable should check if variable already exist? #234

Closed
maecry opened this issue Jul 30, 2023 · 6 comments
Closed

AddVariable should check if variable already exist? #234

maecry opened this issue Jul 30, 2023 · 6 comments

Comments

@maecry
Copy link
Contributor

maecry commented Jul 30, 2023

this should have a valid check for exist variable

if (const auto index = GetVariableIndex(uNameHash); index != C_INVALID_VARIABLE)
{
    // does the type match?
    if (vecVariables[index].uTypeHash == uTypeHash)
    { 
        // return the exist index
        return index;
    }
}

could have typos as I typed on github
note: not really a big deal as for now but If anyone wanted to implement some Lua or JS binding, they prob fucked up like I did so this might be helpful

@maecry
Copy link
Contributor Author

maecry commented Jul 30, 2023

also same with this which is for array

@rollraw
Copy link
Owner

rollraw commented Jul 30, 2023

actually it was designed to be used only internally and not interact with the end user in any way, since compiler will not let you register same variable twice inside 'variables.h', so it's fine for now. btw the configuration system is still under maintenance and may be changed in the future

@maecry
Copy link
Contributor Author

maecry commented Jul 30, 2023

alright, nice to hear, thanks, also I noticed this line, static here is better? as I don't see the reason of casting __readfsdword and storing the peb ptr multiple times

@maecry
Copy link
Contributor Author

maecry commented Jul 30, 2023

so far we only call MEM::GetModuleBaseHandle() from the same thread It should work fine without changing the __readfsdword(). correct me tho

@rollraw
Copy link
Owner

rollraw commented Jul 30, 2023

alright, nice to hear, thanks, also I noticed this line, static here is better? as I don't see the reason of casting __readfsdword and storing the peb ptr multiple times

i'm not sure does it changes during runtime, and since it's pretty cheap single instruction it's better to keep it like that

so far we only call MEM::GetModuleBaseHandle() from the same thread It should work fine without changing the __readfsdword(). correct me tho

it's not for thread-safety there, just to grab process base address when passed null to name argument, and follow original winapi method behaviour

@maecry
Copy link
Contributor Author

maecry commented Jul 30, 2023

Thanks for ur explaining, I will close this issue

@maecry maecry closed this as completed Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants