Undo/Redo yielding unexpected results toward plugin actions

[KrimsonWoIf]

General Info
The [ChangedHistoryService] (ChangeHistoryService | Documentation - Roblox Creator Hub), responsible for undo/redo functionality in Roblox Studio, experiences issues when undoing the actions of plugins. The issue can be reproduced universally on all Roblox experiences and does not require reproduction files or system information.

Reproduction of issue

1. Open any Roblox experience within Roblox.
2. Run a plugin that creates or removes parts.
3. Manually move a part right after the plugin does these actions.
4. Undo the actions

Result (main bug)
Both the part you manually moved and the actions done by the plugin will be undone.

Main concerns
Many plugins, particularly those that manage critical and delicate information, such as its files, when users do actions, can have their actions undone unless multiple manual interactions have been done. This can lead to data loss within plugins, which can make them unstable and/or delete developer progress within the realm of the plugin.

Likely reason this happens
Its likely that ChangedHistoryService has issues with indexing the redo/undo actions once a plugin does an action. I believe it may be merging the actions done by the plugin with the manual action afterward as a single index, so that when the developer undoes them, it considers them both as the same action.

Test plugin
I have gone ahead and created this basic plugin to help you reproduce the issue.

1. Copy and paste the code into a script.
2. Right-click the script and select “save as local plugin.”
3. Click the plugin button within the “plugins” section of Roblox named “test bug.”
4. Manually move a separate part and watch it merge both actions by you and the plugin as one.

local toolbar = plugin:CreateToolbar("test bug")
local pluginButton = toolbar:CreateButton("test bug", "test bug", "rbxthumb://type=Asset&id=6810376207&w=150&h=150")
pluginButton.Click:Connect(function()
	local part = Instance.new("Part")
	part.Name = "bugTest"
	part.Parent = workspace
	part.Size = Vector3.new(5,5,5)
	warn("a part has been created at 0,0,0. Please inspect and review ChangedHistoryService's behavior.")
end)

BASIC EXAMPLE

1. I clicked the plugin button, creating a block by the plugin.
2. I duplicated baseplate to represent a manual action.
3. I pressed control Z to undo the action.
4. Roblox merged these actions together as the index never increased, causing them both to be undone when I Pressed Control Z.

its highly likely that ChangedHistoryService records plugin actions, but only increases the index within ChangedHistoryService actions that are done manually by developers, causing plugin actions to share the same index as manual interactions.

SIGNIFICANT DATA LOSS

1. I created many tasks using my task creator plugin. The plugins stores user data by creating folders, which gets stored within ChangedHistoryService under the same index as roblox doesnt increase the index for plugin actions
2. I duplicated baseplate to represent a manual interaction.
3. I pressed control Z to undo the baseplate deletion, but it also deleted all of the work I just did within my plugin.

This is an example of how this issue can significantly affect developers experiences in the realm of plugins, leading to significant data loss and progress.

[ChipioIndustries (chipio)]

a) The ROBLOXCRITICAL label hasn’t been used in years. There’s no point in adding it to your title.
b) There’s no bug here, you’re using plugins that haven’t implemented ChangeHistoryService. It’s on plugin developers to make sure the tools they make properly support the undo-redo system.

Here’s how your example code might look with ChangeHistoryService integration:

local ChangeHistoryService = game:GetService("ChangeHistoryService")

local INTERNAL_NAME = "insertPart"
local RECORDING_NAME = "Insert Part"

local toolbar = plugin:CreateToolbar("test bug")
local pluginButton = toolbar:CreateButton("test bug", "test bug", "rbxthumb://type=Asset&id=6810376207&w=150&h=150")
pluginButton.Click:Connect(function()
	local recordingIdentifier = ChangeHistoryService:TryBeginRecording(INTERNAL_NAME, RECORDING_NAME)
	if recordingIdentifier then
		local part = Instance.new("Part")
		part.Name = "bugTest"
		part.Parent = workspace
		part.Size = Vector3.new(5,5,5)
		ChangeHistoryService:FinishRecording(recordingIdentifier)
		warn("a part has been created at 0,0,0. Please inspect and review ChangedHistoryService's behavior.")
	else
		wart("Studio is currently busy. Please try again.")
	end
end)

note: I did not test this and have not yet used the new recordings system so this may not work quite right

If you use a plugin that loses its changes when you undo a different operation, you should contact the developer and ask them to implement these methods.

[KrimsonWoIf]

I ran and debuged your code. I chanegd “INTERAL_NAME” and “RECORDING_NAME” values to be “test bug” (name of plugin) instead of “Insert part” (gives an error).

When I tried running the plugin it repeated Studio is currently busy. Please try again.

[KrimsonWoIf]

I watched a youtube video made by you and I did the following:

local ChangeHistoryService = game:GetService("ChangeHistoryService")
local toolbar = plugin:CreateToolbar("test bug")
local pluginButton = toolbar:CreateButton("test bug", "test bug", "rbxthumb://type=Asset&id=6810376207&w=150&h=150")
pluginButton.Click:Connect(function()
	
	local part = Instance.new("Part")
	part.Name = "bugTest"
	part.Parent = workspace
	part.Size = Vector3.new(5,5,5)
	ChangeHistoryService:SetWaypoint("Part creation")
	warn("a part has been created at 0,0,0. Please inspect and review ChangedHistoryService's behavior.")
	
end)

I guess since it was your youtube video you made you got the solution. I am surpised you forgot how to use it though and thanks for the video.

[ChipioIndustries (chipio)]

SetWaypoint is outdated, which is why I didn’t recommend it. It works for now but I would try to get TryBeginRecording working correctly to guarantee it working into the future.

[music_man1996]

We’ve found someone to look at this, we’ll update you as soon as we can

[tnavarts]

As noted above, not a bug.

There’s no reliable automatic way for us to identify whether something a plugin is doing a permanent change resulting from a user invoked action or an internal / temporary / visual only change which doesn’t need to be recorded.

The only reliable way for us to know is for the plugin to tell us, by calling Begin / End recording marking which changes are part of an action.

This is actually one of the advantages of the newer TryBeginRecording API over the older SetWaypoint API, once most plugin developers have moved to the newer API we’ll be able to begin warning plugin developers when they failed to call TryBeginRecording before making changes.

[KrimsonWoIf]

I understand your concern about there not being a reliable method for detecting changes made by plugins and I agree with your solution. I think the biggest thing now is:

1. Making it easier for developers to access the functions related to game:GetHistoryService. An example is possibly implemented it into the plugin section to show that these functions are intended for plugins as well as making the functions easier.

An example is

plugin:SetHistoryWaypoint()

I believe the paramter is not nessesary and you shouldnt have to “start recording” or “end recording” it should just undo all the actions before they call this function. This makes it more accessible for much more developers and easier to understand.

2. Spreading awareness of this function. Through announcements or other programs, it would be valueable to spread awareness of this as a way for more plugins to acess this critical feature.

[tnavarts]

The TryBeginRecording pattern is necessary complexity. Yes, a single waypoint call is simpler but it leads to worse UX long term:

• It doesn’t give us enough info about the developer’s intent so that we can do things like avoid autosaving in the middle of a plugin taking an action.

• It doesn’t give the developer enough info about our intent so that they can avoid taking actions while Studio is busy.

We will be looking at making more attempts to call it out for the developer when they miss using the change history APIs but the existing APIs work as intended.

[KrimsonWoIf]

Hello, thank you for your response. I appreciate the existing functionality of historyservice and your dedication to making it work well in situations like autosaving. I have a few questions however. For one, what is the point of all the parameters, which may come off as confusing for many developers?You mentioned that roblox needs this complexity to understand when to save, so is roblox using NLP language models to make decisions for history service or something (particullarly for the confusing string parameters)? Maybe it can be considered whether some parameters can be merged or removed to reduce complexity, to reduce the learning curve of these functions especially for plugins.

For example, names and display names can seem like an extra step. There are mentioned coding and logging purposes but I’m having trouble figuring out how this would work.

what is the purpose of adding Enum.FinishRecordingOperation.Commit considering there is no other uses for FinishRecording() than to finish recordings?

I also found many different issuses with trying to record different parts of the code and I ended up having to merge a bunch of code into one thing (since I cant have multple startRecordings without facing issues within the same code even if these startrecordings play and end respectively to each other.)

Lastly, I would like to mention the ability to lock HistoryService actions alltogeather so that it may not influence actions by a plugin. An example is my plugin which manages data for itself. I do not want undo functionality undoing critical actions that my plugin does to manage its own data, and being able to exclude such actions seems appropriate, especially considering that we can already lock the entire history service.

[tnavarts]

@KrimsonWoIf

For example, names and display names can seem like an extra step. There are mentioned coding and logging purposes but I’m having trouble figuring out how this would work.

Just think name = an fixed identifier for coding purposes, displayName = the localized name which should show up in the UI, ideally in the user’s chosen language if the plugin has been localized.

@KrimsonWoIf

what is the purpose of adding Enum.FinishRecordingOperation.Commit considering there is no other uses for FinishRecording() than to finish recordings?

There are two additional options:

• Cancel - Automatically undoes all of the things you’ve done so far since beginning the recording.
• Append - Merges the current recording with the previous one.