Skip to content

Fixed a problem with Boolean in CFunction::Call.#419

Merged
jordanbriere merged 4 commits into
Source-Python-Dev-Team:masterfrom
CookStar:fix_bool_function_call
Sep 25, 2021
Merged

Fixed a problem with Boolean in CFunction::Call.#419
jordanbriere merged 4 commits into
Source-Python-Dev-Team:masterfrom
CookStar:fix_bool_function_call

Conversation

@CookStar

@CookStar CookStar commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

When testing is_in_field_of_view(#417), the results were incorrect(#316), so I looked into it and found that the boolean was judging the whole value of eax (e.g. -27648 was True), which seems to be giving incorrect results.

I don't think this will break any existing plugins, but I can't be sure.

Test Code:

#   Memory
from memory import Convention
from memory import DataType
#   Players
from players.entity import Player

player = Player(1)
player2 = Player(2)

is_in_field_of_view  = player.make_virtual_function(
    276,
    Convention.THISCALL, (
        DataType.POINTER,
        DataType.POINTER,
    ),
    DataType.INT
)

print(player.is_in_field_of_view(player2.origin))
print(is_in_field_of_view(player, player2.origin) & 1)
print("int is_in_field_of_view", is_in_field_of_view(player, player2.origin))

@jordanbriere

Copy link
Copy Markdown
Contributor

Hmmm. Wouldn't this be a better solution?

return object(CallHelper<bool>(dcCallChar, g_pCallVM, m_ulAddr));

This would ensure we only read one byte, and that everything non-zero still evaluate to true. When you do & 1, every even numbers evaluate as false, which may causes random issues and is inconsistent with the compilers when it comes to intbool conversion.

@Ayuto

Ayuto commented Sep 24, 2021

Copy link
Copy Markdown
Member

I would just properly configure DynCall. By default boolean is handled as int:
https://github.com/Snaipe/dyncall/blob/51e79a84fd91881d7424b28271c6dda4e0d97c11/dyncall/dyncall_config.h#L42
https://github.com/Snaipe/dyncall/blob/51e79a84fd91881d7424b28271c6dda4e0d97c11/dyncall/dyncall_types.h#L49

@jordanbriere

Copy link
Copy Markdown
Contributor

Even better! Assuming it doesn't change the alignment when pushing arguments though.

@CookStar

Copy link
Copy Markdown
Contributor Author

This would ensure we only read one byte, and that everything non-zero still evaluate to true. When you do & 1, every even numbers evaluate as false, which may causes random issues and is inconsistent with the compilers when it comes to intbool conversion.

You're right, it was a bit of a thoughtless fix.

I would just properly configure DynCall. By default boolean is handled as int:
https://github.com/Snaipe/dyncall/blob/51e79a84fd91881d7424b28271c6dda4e0d97c11/dyncall/dyncall_config.h#L42
https://github.com/Snaipe/dyncall/blob/51e79a84fd91881d7424b28271c6dda4e0d97c11/dyncall/dyncall_types.h#L49

Now I see that this is the source of the problem.
Changed dyncall_config.h, thanks Ayuto!

@jordanbriere

Copy link
Copy Markdown
Contributor

Ran some tests and it works. The arguments are also not affected because bool, char, short and long are all pushed as integers internally for the architectures we support which also mean we don't have to recompile DynCall. The following snippets used to assert on the return value being True rather than False:

from memory import *

@Callback(Convention.CDECL, (DataType.INT, DataType.BOOL, DataType.INT, DataType.BOOL), DataType.BOOL)
def callback(stack_data):
    assert tuple(stack_data) == (1, True, 2, False)
    return False

assert not callback(1, True, 2, False)

But now works as intended with this branch.

@jordanbriere jordanbriere merged commit 45b24d8 into Source-Python-Dev-Team:master Sep 25, 2021
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.

3 participants