Clear IP address textbox after clicking invite - #3566
Conversation
|
that's some very complicated code.... I don't know if even quantum can understand that... I think you will have to split that into 100 parts \s |
|
ill make sure to add at least 1 line of comment per letter/s but does this look good? i copied the clear thing from the chat input code not sure if its also applicable here or if the line is in the right part of the function |
| if (err != error.AlreadyConnected) { | ||
| std.log.err("Cannot connect user: {s}", .{@errorName(err)}); | ||
| } | ||
| ipAddressEntry.clear(); |
There was a problem hiding this comment.
this is not the correct place. THis would only give feedback if the user is already connected or another error appeard. The issue is describing that there should always be visual feedback
There was a problem hiding this comment.
ok, i put it in the front of the function, i think this is the right place but honestly I mainly have no clue what that "_thread" thing is
There was a problem hiding this comment.
ok i was informed (by MrPiedPiper) this is not the way to go as we do actually need to use ipadressentry and i have to store it
| } | ||
|
|
||
| fn invite() void { | ||
| ipAddress = ipAddressEntry.currentString.items; |
There was a problem hiding this comment.
ipAddressEntry.currentString.items is now empty because you cleared it.
Please actually test the invite functionality and check in the player list if it actually invited the right IP address.
There was a problem hiding this comment.
sorry, meant to put ipAddress there, i currently just test it out on the client and then copy the code to the github website so i should probably figure out how to push a change from vscode. im gonna look at the code some more to see if ipAddress is ok to use to store the entry here.
There was a problem hiding this comment.
This is still the same issue, you are clearing the memory, yet you are still using a reference to it (const storeIpAddressEntry = ipAddressEntry.currentString; is making a copy of the pointer, not the underlying memory).
This is working in your test because in reality clear doesn't change the memory, but this is an implementation detail and you are just lucky that it worked.
Since you are just starting out, I'd recommend to maybe first do something like ziglings so you understand all the basics before contributing to Cubyz and stumbling over how Zig handles memory differently from other languages.
|
ok im decently confident its good now |
Wunka
left a comment
Wunka
left a comment
There was a problem hiding this comment.
ok im decently confident its good now
what does confident mean? I don't want to sound harsh, but have you actually now tested the code?
| const storeIpAddressEntry = ipAddressEntry.currentString; | ||
| ipAddressEntry.clear(); |
There was a problem hiding this comment.
please read into what you are actually doing here.
As quantum said, it doens't really make sense and makes very wierd behavior possible.
i really really hope this is right.
IntegratedQuantum
moved this from In review
to WIP/not ready for review
in PRs to review
3 participants
babies first zig line
attempt at fixing #3485