godot-nim / gdext-nim

Godot GDExtension binding for Nim
MIT License
27 stars 2 forks source link

Add-ons can lock-up without any announcement #25

Closed insomniacUNDERSCORElemon closed 1 month ago

insomniacUNDERSCORElemon commented 1 month ago

/I have found a case where a LineEdit's text_changed signal causes the add-on to stop functioning completely if the input text is 20 characters or more. The add-on does not recover if said case is undone. There does not seem to be any error output to the Godot console or debug,

Note that if multiple add-ons are present, only the relevant add-on is affected. In other words if 2 classes are in the same addon, errors like this affect both.

This is not a hard limit, I have an older version of code that does not exhibit this exact issue. However, in this case it still does lock up if I reach the maximum condition (64 characters) that I already have set in my code. If I move that condition up (it is already 2nd) it prevents that specific lock-up so it seems to happen when checking conditions (/branching?), as it seems like this block evaluating first:

  if nonhex_chars in intext or intext.len == 0:
    self.disable_all_buttons("Waiting for valid input.")
    return

Should not be causing an issue. If a related error did happen in the 3.X equivalent it was not visible to the user. EDIT: I am not seeing this exact issue (64 chars) when testing anymore, I'm not sure if I'm doing something wrong or if there's something else going on here. But lock-up and lack of errors is still an issue.

Related: decbinhex demo, main_class.nim Possibly related: ExtensionManager

panno8M commented 1 month ago

please add following sentence for each extensions' config.nims and retry:

switch("define", "projectName:" & projectName())
insomniacUNDERSCORElemon commented 1 month ago

please add following sentence for each extensions' config.nims and retry

I updated and did that+recompiled both (and tried different line placement for that), seems to be no change. Assuming gdextwiz is properly updating.

Also I thought maybe the lock-up itself was caused by my code so I re-wrote inbox_text_changed with the length calculation as a case statement. I think I successfully covered every case, and it's still breaking if I type 20 ones even though it absolutely shouldn't. EDIT: It could be that I couldn't figure out how to do a case statement with in. If I do 20 twos instead, I can see that the proc breaks somewhere after the length calculation.

Just to be clear, this issue isn't caused by multiple add-ons though the issue that is doesn't seem to be fixed by it either.

insomniacUNDERSCORElemon commented 1 month ago

Alright, I just found the source of what's causing the procedure to break:

checking if intext.parseInt == 0 on a string that is 20 characters or longer.

with import strutils and const break_string:string = "123456789012345678901" put this in ready and the add-on will be broken: if break_string.parseInt == 0: print "hmm"

If put into dodgethecreeps in player.nim, it won't even start.

EDIT: Which is an error specific to parseInt, not reported anywhere I can see though. I just checked and I originally had a check for that and forgot that's why I needed it.

panno8M commented 1 month ago

It seems that this library has broken Nim's exception mechanism somewhere.