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

issue_360 Fix dialog_caption size #361

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,40 @@ extends PopochiuDialogText
#region Private ####################################################################################
func _modify_size(msg: String, _target_position: Vector2) -> void:
var _size := await _calculate_size(msg)

# Define size and position (before calculating overflow)
rich_text_label.size.x = _size.x
rich_text_label.size.y = _size.y
rich_text_label.position.x = (get_viewport_rect().size.x/2) - (_size.x /2)
Comment on lines +9 to +11
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the ideal approach in this case is not to modify the width (size.x) and X position of the RichTextLabel. Considering how this Control is configured in Godot, what we should do is ensure that the value defined in the wrap_width property is assigned to the size.x property of the RichTextLabel in the _ready() function. To control the Y position of the component, Godot's anchors can be used.

rich_text_label.position.y = get_meta(DFLT_POSITION).y - (_size.y - get_meta(DFLT_SIZE).y)


func _append_text(msg: String, props: Dictionary) -> void:
msg = _correct_line_breaks(msg)
rich_text_label.text = "[center][color=%s]%s[/color][/center]" % [props.color.to_html(), msg]


## Appends text for the dialog caption
## Ensures that where a printing a word would see it wrap to the next line that the newline
## is forced into the text. Without this the tween in dialog_text.gd would print part of the word
## until it runs out of space, then erase the part word and rewrite it on the next line which looks
## messy.
func _correct_line_breaks(msg: String) -> String:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think _fix_line_breaks is a better name for this function. "Correct" sounds weird.

rich_text_label.text = msg
var number_of_lines_of_text := rich_text_label.get_line_count()
if number_of_lines_of_text > 1:
var current_line_number := 0
for current_character in range(0, rich_text_label.text.length()):
if rich_text_label.get_character_line(current_character) > current_line_number:
current_line_number += 1
if rich_text_label.text[current_character-1] == " ":
rich_text_label.text[current_character-1] = "\n"
elif rich_text_label.text[current_character-1] != "\n":
rich_text_label.text = rich_text_label.text.left(current_character) +\
"\n" + rich_text_label.text.right(-current_character)

if current_line_number == number_of_lines_of_text - 1:
break
return rich_text_label.text
Comment on lines +25 to +41
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like what this code does, but it shouldn't be duplicated in the 3 DialogText components. You should move this to the dialog_text.gd script so any DialogText component can make use of it.


#endregion
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ extends PopochiuDialogText
#region Private ####################################################################################
func _modify_size(msg: String, target_position: Vector2) -> void:
var _size := await _calculate_size(msg)

# Define size and position (before calculating overflow)
rich_text_label.size = _size
rich_text_label.position = target_position - rich_text_label.size / 2.0
Expand All @@ -27,13 +26,13 @@ func _set_default_label_size(lbl: Label) -> void:


func _append_text(msg: String, props: Dictionary) -> void:
msg = _correct_line_breaks(msg)

Comment on lines +29 to +30
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assign the [color][/color] to the msg in the same way you did for the dialog_caption.gd:17 and the dialog_portrait.gd:47.

var center: float = floor(rich_text_label.position.x + (rich_text_label.size.x / 2))
if center == props.position.x:
rich_text_label.text = "[center]%s[/center]" % msg
elif center < props.position.x:
rich_text_label.text = "[right]%s[/right]" % msg
else:
rich_text_label.text = msg


func _get_icon_from_position() -> float:
Expand All @@ -44,4 +43,28 @@ func _get_icon_to_position() -> float:
return rich_text_label.size.y / 2.0 + 3.0


## Appends text for the dialog caption
## Ensures that where a printing a word would see it wrap to the next line that the newline
## is forced into the text. Without this the tween in dialog_text.gd would print part of the word
## until it runs out of space, then erase the part word and rewrite it on the next line which looks
## messy.
func _correct_line_breaks(msg: String) -> String:
rich_text_label.text = msg
var number_of_lines_of_text := rich_text_label.get_line_count()
if number_of_lines_of_text > 1:
var current_line_number := 0
for current_character in range(0, rich_text_label.text.length()):
if rich_text_label.get_character_line(current_character) > current_line_number:
current_line_number += 1
if rich_text_label.text[current_character-1] == " ":
rich_text_label.text[current_character-1] = "\n"
elif rich_text_label.text[current_character-1] != "\n":
rich_text_label.text = rich_text_label.text.left(current_character) + "\n" +\
rich_text_label.text.right(-current_character)

if current_line_number == number_of_lines_of_text - 1:
break
return rich_text_label.text


Comment on lines +46 to +69
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this after moving this function to the dialog_text.gd script.

#endregion
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,32 @@ func _set_default_size() -> void:
pass


func _append_text(msg: String, props: Dictionary) -> void:
msg = _correct_line_breaks(msg)
rich_text_label.text = "[color=%s]%s[/color]" % [props.color.to_html(), msg]


## Appends text for the dialog caption
## Ensures that where a printing a word would see it wrap to the next line that the newline
## is forced into the text. Without this the tween in dialog_text.gd would print part of the word
## until it runs out of space, then erase the part word and rewrite it on the next line which looks
## messy.
func _correct_line_breaks(msg: String) -> String:
rich_text_label.text = msg
var number_of_lines_of_text := rich_text_label.get_line_count()
if number_of_lines_of_text > 1:
var current_line_number := 0
for current_character in range(0, rich_text_label.text.length()):
if rich_text_label.get_character_line(current_character) > current_line_number:
current_line_number += 1
if rich_text_label.text[current_character-1] == " ":
rich_text_label.text[current_character-1] = "\n"
elif rich_text_label.text[current_character-1] != "\n":
rich_text_label.text = rich_text_label.text.left(current_character) + "\n" +\
rich_text_label.text.right(-current_character)

if current_line_number == number_of_lines_of_text - 1:
break
return rich_text_label.text

Comment on lines +50 to +72
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this after moving this function to the dialog_text.gd script.

#endregion
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,18 @@ func play_text(props: Dictionary) -> void:

if PopochiuConfig.should_talk_gibberish():
msg = D.create_gibberish(msg)

# Call the virtual method that modifies the size of the RichTextLabel in case the dialog style
# requires it.
await _modify_size(msg, props.position)

rich_text_label.push_color(props.color)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We removed this in #357 because it was causing issues when centering text. Please remove it again since each DialogText component should assign the color itself using BBCode ([color][/color]).


# Assign the text and align mode
msg = "[color=%s]%s[/color]" % [props.color.to_html(), msg]
balloonpopper marked this conversation as resolved.
Show resolved Hide resolved
_append_text(msg, props)
if PopochiuConfig.should_talk_gibberish():
_append_text(D.create_gibberish(msg), props)
else:
_append_text(msg, props)
Comment on lines +84 to +87
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be just this since the gibberish text replacement was already done in line 75.

Suggested change
if PopochiuConfig.should_talk_gibberish():
_append_text(D.create_gibberish(msg), props)
else:
_append_text(msg, props)
_append_text(msg, props)


if _secs_per_character > 0.0:
# The text will appear with an animation
Expand Down Expand Up @@ -169,46 +173,37 @@ func _modify_size(_msg: String, _target_position: Vector2) -> void:


## Creates a RichTextLabel to calculate the resulting size of this node once the whole text is shown.
## Uses a RichTextLabel to provide the "get_parsed_text" function, and a label within it to work out
## the minimum size. Calculating the size from just the RichTextLabel does not work.
Comment on lines 175 to +177
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to replace the description of the function, please remove the previous one. Also, I suggest a small change to make it clear what the function really does.

Suggested change
## Creates a RichTextLabel to calculate the resulting size of this node once the whole text is shown.
## Uses a RichTextLabel to provide the "get_parsed_text" function, and a label within it to work out
## the minimum size. Calculating the size from just the RichTextLabel does not work.
## Uses a [RichTextLabel] and a [Label] to work out the minimum size since calculating it from just
## the [RichTextLabel] does not work.

func _calculate_size(msg: String) -> Vector2:
var rt := RichTextLabel.new()
rt.add_theme_font_override("normal_font", get_theme_font("normal_font"))
rt.bbcode_enabled = true
rt.autowrap_mode = TextServer.AUTOWRAP_WORD_SMART
rt.text = msg
rt.size = get_meta(DFLT_SIZE)
rich_text_label.add_child(rt)

# Create a Label to check if the text exceeds the wrap_width
var lbl := Label.new()
lbl.add_theme_font_override("normal_font", get_theme_font("normal_font"))

_set_default_label_size(lbl)

lbl.text = rt.get_parsed_text()
lbl.size = lbl.get_minimum_size()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is not necessary since the custom_minimum_size of the component at this point will be Vector2.ZERO.

rich_text_label.add_child(lbl)

rt.clear()
rt.text = ""

await get_tree().process_frame

var _size := lbl.size

if _size.x > wrap_width:
# This node will have the width of the wrap_width
# This node will have the width of the wrap_width.
# The size.y value will calculate automatically.
_size.x = wrap_width
rt.fit_content = true
rt.size.x = _size.x
rt.text = msg
await get_tree().process_frame


# Size is recalculated after the frame changes
await get_tree().process_frame
_size = rt.size
else:
# This node will have the width of the text
_size.y = get_meta(DFLT_SIZE).y

var characters_count := lbl.get_total_character_count()

lbl.free()
rt.free()
Expand Down