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

Bind the String::humanize_size method #32546

Merged
merged 1 commit into from
Oct 4, 2019
Merged

Bind the String::humanize_size method #32546

merged 1 commit into from
Oct 4, 2019

Conversation

Xrayez
Copy link
Contributor

@Xrayez Xrayez commented Oct 4, 2019

As suggested in #31993:

We could also consider exposing this method to scripts, so that projects that need to display file sizes can use it directly.

Exactly my use case. I want to compare different compression methods more easily.

The method signature is also changed to use uint64_t instead of size_t for it to be Variant-compatible: #32546 (comment).

@Xrayez Xrayez requested a review from a team as a code owner October 4, 2019 11:39
@akien-mga akien-mga added this to the 3.2 milestone Oct 4, 2019
@Xrayez
Copy link
Contributor Author

Xrayez commented Oct 4, 2019

Fails on macOS according to Travis:

core/variant_call.cpp:287:2: error: conversion from 'const Variant' to 'size_t' (aka 'unsigned long') is ambiguous
        VCALL_LOCALMEM1R(String, humanize_size);

These two would resolve this I think, but they're not compiled by default it seems:

godot/core/variant.h

Lines 175 to 178 in 82b9557

#ifdef NEED_LONG_INT
operator signed long() const;
operator unsigned long() const;
#endif

iphone platform does include this NEED_LONG_INT flag:

env.Append(CPPDEFINES=['NEED_LONG_INT'])

@akien-mga
Copy link
Member

akien-mga commented Oct 4, 2019

I'd just change humanize_size to use uint64_t.

The method signature is also changed to use `uint64_t` instead of `size_t`
for it to be Variant-compatible.
@Xrayez Xrayez changed the title Bind String::humanize_size method Bind the String::humanize_size method Oct 4, 2019
@Xrayez Xrayez requested a review from akien-mga October 4, 2019 13:58
@akien-mga akien-mga merged commit 11bbe15 into godotengine:master Oct 4, 2019
@akien-mga
Copy link
Member

Thanks!

@Xrayez Xrayez deleted the bind-string-humanize-size branch October 4, 2019 14:05
@aaronfranke
Copy link
Member

What's the reason for NEED_LONG_INT existing? Isn't long long almost always 64-bit and int almost always 32-bit? This code existed before Godot was open source, so cc @reduz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants