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

System search bar #113

Closed
wants to merge 13 commits into from
Closed

System search bar #113

wants to merge 13 commits into from

Conversation

Nidoxs
Copy link
Contributor

@Nidoxs Nidoxs commented Aug 14, 2024

Proposed changes

Adds a search bar to the client debugger view. Systems are hidden when they do not match the search filter.

Related issues

Closes #109

Additional comments

The server debugger would also benefit greatly from the search feature but the way that's all setup makes it more complicated. Let's re-visit another time to add the search to the server view.

Preview

https://gyazo.com/509ee4478fa14ac37a1b7ad642b128df
357981913-1aa9573b-f8bc-4146-bc3c-4652bc3c7168
357981922-0f8b2cd4-e44f-493a-9ca6-1162ec3091ab

lib/debugger/ui.luau Outdated Show resolved Hide resolved
lib/debugger/ui.luau Outdated Show resolved Hide resolved
@jackTabsCode
Copy link
Contributor

I'm not sure it makes sense to add it to one realm but not the other

@Nidoxs
Copy link
Contributor Author

Nidoxs commented Aug 15, 2024

I'm not sure it makes sense to add it to one realm but not the other

I do agree that it is peculiar to add it to one realm and not the other. The shared server debugger complicates implementing the search feature, but I'm happy to explore that again with guidance.

@LastTalon
Copy link
Member

but the way that's all setup makes it more complicated.

Can you explain in more detail exactly what is making it more complicated and how much more complicated?

@Nidoxs
Copy link
Contributor Author

Nidoxs commented Aug 15, 2024

Do we share the search filter between users or do we come up with a way to allow users to search independently from each other?

@LastTalon
Copy link
Member

Do we share the search filter between users or do we come up with a way to allow users to search independently from each other?

Right now the there's just one server debugger, so it doesn't really seem like that's much different from other parts of the state that get shared for everyone using it unless I'm misunderstanding something.

@Nidoxs
Copy link
Contributor Author

Nidoxs commented Aug 15, 2024

but the way that's all setup makes it more complicated.

Can you explain in more detail exactly what is making it more complicated and how much more complicated?

The server debugger is setup in a way that shares the debugger state between debugging clients. The EventBridge class seems to handle parts of that behaviour. I was unsure on how best to approach implementing it, as following the existing shared behaviour of the server debugger would mean that the search filter is shared between debugging clients. Essentially, if multiple clients were typing in the server search bar at the same time, their search filters would be clashing with each other.

I could have another look at some point to see if making the search on the server unique to the client who is searching is feasible with the current setup. I'm not yet well familiarized with the shared server debugger's logic (the part of it that shares the state between all debugger clients), so that may be why I found it less straightforward.

@Nidoxs
Copy link
Contributor Author

Nidoxs commented Aug 15, 2024

image

I did try adding a updateSearchFilter condition case here, but there's more to it than that.

@LastTalon
Copy link
Member

as following the existing shared behaviour of the server debugger would mean that the search filter is shared between debugging clients.

Right, this is what I'm saying. This doesn't sound like new behavior or a new challenge to me, it sounds like it's just baggage we already deal with in the server debugger because of the way it works. There's lots of state that's sort of weird because of how it operates in this shared way, and this just sounds like another.

We can improve it, but I think it's just okay to have it work that way for now, and we can look into improving it later. Honestly, most of the time only one person is really using the debugger at a time anyway. So it's a bit of unpleasantness sometimes, but not something that I think really should prevent it from functioning.

If you think you have a good handle on the problem you can go ahead and pre-emptively make an issue for it, but I think we should have this working in the server debugger even if it behaves weird when that happens.

@Nidoxs
Copy link
Contributor Author

Nidoxs commented Aug 15, 2024

I'll take a look at the server search functionality again! 😄

@Nidoxs Nidoxs closed this Aug 15, 2024
@jackTabsCode
Copy link
Contributor

@Nidoxs Why is this closed? I think that it's currently OK to have shared state between client and server in the debugger because that's how the rest of this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add search bar to debugger's system list
4 participants