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

Profiling support #174

Merged
merged 1 commit into from
Nov 27, 2018
Merged

Conversation

marcelofg55
Copy link
Contributor

@marcelofg55 marcelofg55 commented Aug 20, 2018

This can be added now that godotengine/godot#21229 was merged.

Note: To make functions send profiling data to godot use GODOT_PROFILING_FUNCTION (DEBUG_ENABLED must be declared on the SConstruct) on each function, like this:

void Player::TestFunctions() {
    GODOT_PROFILING_FUNCTION

    // actual code after GODOT_PROFILING_FUNCTION
}

@marcelofg55 marcelofg55 changed the title Profiling support [WIP] Profiling support Aug 26, 2018
@karroffel
Copy link
Contributor

This needs to be updated to reflect the name changes done in the Godot side, then it should be good to go :)

@marcelofg55
Copy link
Contributor Author

This needs to be updated to reflect the name changes done in the Godot side, then it should be good to go :)

I've realized this needs NativeScript 1.1 now, after I moved the godot_nativescript_profiling_add_data function its now part of godot_gdnative_ext_nativescript_1_1_api_struct. So I'll wait for the NativeScript 1.1 to be merged to master.

@BastiaanOlij
Copy link
Collaborator

@marcelofg55 nativescript 1.1 has been merged, can you check if this is still valid and if so update the PR?

@marcelofg55
Copy link
Contributor Author

@BastiaanOlij I'll convert my project to nativescript 1.1 and then re-check this. Are the GDNative demos updated to nativescript 1.1?

@marcelofg55
Copy link
Contributor Author

@BastiaanOlij I have a question regarding nativescript 1.1, hope it's okay if I ask it here. I'm converting my code to nativescript 1.1 and I used to have this code:

	Node *area = (Node*)(Object*)loader->call("getAreaScript");
//	Node *area = (Node*)(godot_object*)loader->call("getAreaScript"); // this compiles on 1.1 but crashes later on ->call
	if (area) {
		if (area->call("isDisableMiniMap")) {
			attenuate = false;
		}
	}

getAreaScript is a GDScript function that returns a Node basically, it worked fine in nativescript 1.0 but now its crashing when I execute a call on area. What's the right way of doing this now with 1.1?

@BastiaanOlij
Copy link
Collaborator

@marcelofg55 not sure, my gut feeling says:

Ref<Node> area = loader->call("getAreaScript");
if (area.is_valid()) {
  if (area->call("isDisableMiniMap")) {
    attenuate = false;
  }
}

Might work, but I could be very wrong.

@marcelofg55
Copy link
Contributor Author

Your suggestion makes sense but its failing to compile:

gcc-5 -o src/sample_player.os -c -fPIC -g -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wno-unused-but-set-parameter -m64 -O0 -DDEBUG_ENABLED -fPIC -I. -Isrc -Iinclude -Igodot-cpp/godot_headers -Igodot-cpp/include -Igodot-cpp/include/core -Igodot-cpp/include/gen src/sample_player.cpp
In file included from godot-cpp/include/core/Godot.hpp:13:0,
                 from include/common.h:7,
                 from include/sample_player.h:4,
                 from src/sample_player.cpp:1:
godot-cpp/include/core/Ref.hpp: In instantiation of 'void godot::Ref<T>::unref() [with T = godot::Node]':
godot-cpp/include/core/Ref.hpp:161:9:   required from 'godot::Ref<T>::Ref(const godot::Variant&) [with T = godot::Node]'
src/sample_player.cpp:50:47:   required from here
godot-cpp/include/core/Ref.hpp:180:17: error: 'class godot::Node' has no member named 'unreference'
   if (reference && reference->unreference()) {
                 ^
godot-cpp/include/core/Ref.hpp: In instantiation of 'void godot::Ref<T>::ref(const godot::Ref<T>&) [with T = godot::Node]':
godot-cpp/include/core/Ref.hpp:168:6:   required from 'godot::Ref<T>::Ref(const godot::Variant&) [with T = godot::Node]'
src/sample_player.cpp:50:47:   required from here
godot-cpp/include/core/Ref.hpp:26:4: error: 'class godot::Node' has no member named 'reference'
    reference->reference();
    ^
scons: *** [src/sample_player.os] Error 1

@marcelofg55 marcelofg55 force-pushed the profiling_data branch 2 times, most recently from 4841daa to b33810b Compare November 21, 2018 00:54
@marcelofg55 marcelofg55 changed the title [WIP] Profiling support Profiling support Nov 21, 2018
@marcelofg55
Copy link
Contributor Author

This is ready for review/merge.

Copy link
Contributor

@karroffel karroffel left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@BastiaanOlij
Copy link
Collaborator

@marcelofg55 looks like you remove nativescript_terminate by accident. If you could fix that up I'll merge these in :)

@marcelofg55
Copy link
Contributor Author

@BastiaanOlij I've just fixed the nativescript_terminate thing on the PR and rebased it to the latest master. I guess the PR should be okay now.

@BastiaanOlij
Copy link
Collaborator

@marcelofg55 it's still showing that it wants to remove the nativescript_terminate method though?

@BastiaanOlij BastiaanOlij added the enhancement This is an enhancement on the current functionality label Nov 27, 2018
@marcelofg55
Copy link
Contributor Author

@BastiaanOlij Just re-fixed that, seems fine now :)

Copy link
Contributor

@karroffel karroffel left a comment

Choose a reason for hiding this comment

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

Looking good now!

@BastiaanOlij BastiaanOlij merged commit 6fb835c into godotengine:master Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants