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

fix: potential infinite loop on DialogicResourceUtil.gd #2421

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AleryBerry
Copy link

@AleryBerry AleryBerry commented Oct 2, 2024

My game was strangely freezing when trying to trigger a timeline. I looked into the codebase and I found out this while loop on DialogicResourceUtil caused the issue.

static func add_resource_to_directory(file_path:String, directory:Dictionary) -> Dictionary:
    var suggested_name := file_path.get_file().trim_suffix("."+file_path.get_extension())
    while suggested_name in directory:
        suggested_name = file_path.trim_suffix("/"+suggested_name+"."+file_path.get_extension()).get_file().path_join(suggested_name)
    directory[suggested_name] = file_path
    return directory

If file_path is user://laura olsdal.dch and directory is { ..., "laura_olsdal": "res://assets/characters/laura_olsdal/laura_olsdal.dch", ... } the code enters an infinite loop. So I just made a little check to break the loop if suggested_name stops changing.

@Jowan-Spooner Jowan-Spooner added Needs testing More feedback/testing would be good Approved This can be added, but is not on any roadmap yet labels Oct 15, 2024
@zaknafean zaknafean self-requested a review October 16, 2024 14:05
Copy link
Collaborator

@zaknafean zaknafean left a comment

Choose a reason for hiding this comment

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

I can't recreate this issue. Anyway you can provide a small project that demonstrates it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved This can be added, but is not on any roadmap yet Needs testing More feedback/testing would be good
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants