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

Refactored Variant setters/getters #43371

Merged
merged 1 commit into from
Nov 7, 2020

Conversation

reduz
Copy link
Member

@reduz reduz commented Nov 7, 2020

  • Discern between named, indexed and keyed
  • Get direct access to functions for typed GDScript and GDNative bindings
  • Small changes to some classes in order to work with the new setget binder

@reduz reduz requested a review from vnen as a code owner November 7, 2020 01:32
core/variant_setget.cpp Outdated Show resolved Hide resolved
core/variant_setget.cpp Outdated Show resolved Hide resolved
core/variant_setget.cpp Outdated Show resolved Hide resolved
@qarmin
Copy link
Contributor

qarmin commented Nov 7, 2020

For me, this build always crash when trying to open editor or project manager - Segmentation fault (core dumped)

Valgrind log(Godot opens 10 minutes)

==84891== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==84891== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==84891== Can't extend stack to 0x1ffe8010a8 during signal delivery for thread 1:
==84891==   no stack segment
==84891== 
==84891== Process terminating with default action of signal 11 (SIGSEGV)
==84891==  Access not within mapped region at address 0x1FFE8010A8
==84891== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==84891==    at 0x1D2E713: HashMap<StringName, ClassDB::ClassInfo, HashMapHasherDefault, HashMapComparatorDefault<StringName>, (unsigned char)3, (unsigned char)8>::get_element(StringName const&) const (hash_map.h:175)
==84891==  If you believe this happened as a result of a stack
==84891==  overflow in your program's main thread (unlikely but
==84891==  possible), you can try to increase the size of the
==84891==  main thread stack using the --main-stacksize= flag.
==84891==  The main thread stack size used in this run was 8388608.
==84891== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==84891== 
==84891== Process terminating with default action of signal 11 (SIGSEGV)
==84891==  Access not within mapped region at address 0x1FFE801F70
==84891== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==84891==    at 0x8807134: _vgnU_freeres (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_core-amd64-linux.so)
==84891==  If you believe this happened as a result of a stack
==84891==  overflow in your program's main thread (unlikely but
==84891==  possible), you can try to increase the size of the
==84891==  main thread stack using the --main-stacksize= flag.
==84891==  The main thread stack size used in this run was 8388608.

@reduz
Copy link
Member Author

reduz commented Nov 7, 2020

@aaronfranke nice catches, thanks! fixing in my local branch as I have some other fixes in the works.

@reduz
Copy link
Member Author

reduz commented Nov 7, 2020

@qarmin fixed the crash locally, I thought everything worked fine but was running an old binary :( will force push with fixes in a bit

@reduz reduz force-pushed the variant-setget-refactor branch from b61c329 to a547fe6 Compare November 7, 2020 10:50
@Calinou Calinou added this to the 4.0 milestone Nov 7, 2020
@qarmin
Copy link
Contributor

qarmin commented Nov 7, 2020

Code

extends Node2D

func _ready() -> void:
	var arr : Array = [25,235236,236,236,23,6,236,236,236,23,634,63,7,35,4734,64,64,34,62,72,32,535]
	arr[-36235 % arr.size()]

shows error Invalid get index '-1'(on base 'Array')

@reduz
Copy link
Member Author

reduz commented Nov 7, 2020

@qarmin good catch! I completely forgot we allow negative indices in arrays!

-Discern between named, indexed and keyed
-Get direct access to functions for typed GDScript and GDNative bindings
-Small changes to some classes in order to work with the new setget binder
@reduz reduz force-pushed the variant-setget-refactor branch from a547fe6 to 05de7ce Compare November 7, 2020 18:16
Copy link
Member

@vnen vnen left a comment

Choose a reason for hiding this comment

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

LGTM

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.

5 participants