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.

Sign up for GitHub

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

Jump to bottom

Removing/Renaming a file causes patch viewer to crash #710

Closed
Dekkonot opened this issue Jul 4, 2023 · 6 comments · Fixed by #713
Closed

Removing/Renaming a file causes patch viewer to crash #710

Dekkonot opened this issue Jul 4, 2023 · 6 comments · Fixed by #713
Assignees
@boatbomber
Labels
scope: plugin Relevant to the Roblox Studio plugin type: bug Something happens that shouldn't happen

Comments

@Dekkonot
Copy link
Member

Dekkonot commented Jul 4, 2023

When renaming a file that's tracked by Rojo, the patch viewer breaks. I'm not sure when this behavior was introduced but it was probably recently.

This can be easily reproduced:

  • Create a new project using rojo init
  • Add a folder or text file to one of the created folders
  • Sync to Studio using the plugin
  • Open patch viewer
  • Rename the previously created file/folder
  • Observe that your plugin is now locked up

If the patch viewer is not open, the rest of the plugin will continue to function but the patch viewer will no longer function.
@Dekkonot Dekkonot added type: bug Something happens that shouldn't happen scope: plugin Relevant to the Roblox Studio plugin labels Jul 4, 2023
@boatbomber
Copy link
Member

boatbomber commented Jul 8, 2023 •
edited
Loading

I have determined the cause. The remove list in a patch contains either string IDs or Instances, but when writing the patch visualizers I forgot and only handled Instances.

Edit: This was the source of the locking up, but even with this fixed the patch viewer still has more problems discovered below that are now tracked in #714.

@boatbomber
Copy link
Member

Oh, this gets tricky. When viewing the patch that was just applied, the instanceMap will no longer have a mapping for that ID, since it was just removed.

@boatbomber
Copy link
Member

This means that removing an instance is not visualizable in this current form. Here's what I get when removing instead of renaming.

image

Renaming is just a remove and an addition.

@boatbomber boatbomber changed the title Renaming a file causes plugin to lock up if patch viewer is open Removing/Renaming a file causes patch viewer to fail Jul 8, 2023
@boatbomber
Copy link
Member

boatbomber commented Jul 8, 2023 •
edited
Loading

The only way to be able to display information that has since been deleted is to store it, but I really don't want Rojo to need to store tree info on every object you have just in case you delete/rename it...

Perhaps the act of deleting could be modified so that when deleting something, we grab its info right before sending it to the abyss. Then that info it given to the patch viewer until the next patch comes in.

@boatbomber
Copy link
Member

I'm going to solve this in two PRs, since the string/instance handling is one fix and the tracking post-removal is a whole other big problem

@boatbomber boatbomber mentioned this issue Jul 8, 2023
Merged
@boatbomber boatbomber changed the title Removing/Renaming a file causes patch viewer to fail Removing/Renaming a file causes patch viewer to crash Jul 8, 2023
@boatbomber
Copy link
Member

boatbomber commented Jul 8, 2023 •
edited
Loading

I created #714 because these are two distinct problems.

Dekkonot pushed a commit that referenced this issue Jul 8, 2023
@boatbomber
Handle strings and instances in patch removes for visualizer (#713)
2735711 
When an object is deleted in a patch, it is either represented with an
ID or an Instance. On initial sync, removals are instances since the map
does not contain those instances. Later removals of managed objects use
an ID. The patch visualizer only handled instances, so this fixes that.

Closes #710.
Dekkonot pushed a commit to UpliftGames/rojo that referenced this issue Jan 11, 2024
@boatbomber
Handle strings and instances in patch removes for visualizer (rojo-rb…
1f01776 
…x#713)

When an object is deleted in a patch, it is either represented with an
ID or an Instance. On initial sync, removals are instances since the map
does not contain those instances. Later removals of managed objects use
an ID. The patch visualizer only handled instances, so this fixes that.

Closes rojo-rbx#710.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@boatbomber boatbomber

Labels
scope: plugin Relevant to the Roblox Studio plugin type: bug Something happens that shouldn't happen
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle strings and instances in patch removes for visualizer boatbomber/rojo
2 participants
@Dekkonot @boatbomber