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

RichTextLabel: Adding ability for single meta hovering #12984

Merged

Conversation

willnationsdev
Copy link
Contributor

This will enable meta_hover_start and meta_hover_end events which also have the meta attached as a parameter. It supports only a single meta tag.

richtextlabel_hoversignals

Note that the use of current_meta for line 930 of the .cpp was only needed because the attached meta kept on printing as an empty array on the scripting side when I tried just using meta_hovering->meta. Not quite sure why. If there is a way to remove the use of current_meta though, I'd be all for it.

@willnationsdev willnationsdev changed the title Adding ability for single meta hovering RichTextLabel: Adding ability for single meta hovering Nov 17, 2017
@willnationsdev willnationsdev force-pushed the richtextlabel-hover-signal branch 3 times, most recently from 7e4b189 to 2a142dd Compare November 17, 2017 06:16
@willnationsdev
Copy link
Contributor Author

not entirely sure if that will affect performance

There shouldn't be any problem since the function in question is already calling _find_click and _find_meta, so it's largely already been devoting those resources.

You might be able to get a hover signal sent out on a recurring basis. Maybe just in the _gui_input, check to see if meta_hovering is non-null? If so, emit with current_meta? I'll try it tomorrow. After midnight for me at the moment.

@akien-mga
Copy link
Member

akien-mga commented Nov 17, 2017

The design of signals is to be discrete, I don't think a meta_hovering signal would make sense as you would be spammed with signals as long as it's true. It should be easy enough to catch meta_hover_start and set a boolean in your code, and reset it when you catch meta_hover_end. This is the design used everywhere in Godot, for example area_entered/area_exited and no area_occupied/overlapped.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Nov 17, 2017

My mistake. Yeah, basically I needed to have access to the item and outside properties, so I had to move the _find_click up out of the if statement. As a result, it will get called much more frequently. So this new code will effectively make the node find the currently clicked item every single time your mouse moves while over the node's dimensions. That will be a loss of performance, but is mandatory to add this feature.

No function caching here (I wish, that'd be a cool feature), just me not picking up on what you meant initially.

Also, I agree with you guys on not including a hovering signal.

When I next get a chance, I'll try to fix the clang format issues. My plugin for it just doesn't seem to work. Will try to use something else or will make the necessary changes manually if need be.

@willnationsdev
Copy link
Contributor Author

Oh woops, and people should note that the signal names are past tense, started and ended.

@willnationsdev willnationsdev force-pushed the richtextlabel-hover-signal branch 2 times, most recently from ffaabc7 to 4a18fb1 Compare November 17, 2017 14:43
@willnationsdev willnationsdev force-pushed the richtextlabel-hover-signal branch from 4a18fb1 to eaea646 Compare November 17, 2017 15:37
@akien-mga akien-mga merged commit d6dc909 into godotengine:master Nov 20, 2017
@HummusSamurai
Copy link
Contributor

Can you post the code for the example in the GIF?

I can't find documentation for this.

@willnationsdev
Copy link
Contributor Author

@HummusSamurai sure.

_ready():
    push_meta("test")
    add_text("hello")
    pop()
    push_meta({"data":"hi"})
    add_text("hello2")
    pop()

@willnationsdev
Copy link
Contributor Author

I've updated documentation in a separate merge as well, so when they next generate the docs, it will have full descriptions.

@HummusSamurai
Copy link
Contributor

@willnationsdev Thank you!

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