So because it errors in a different way, it’s unexpected? every method is a Try, things error for various reasons, this nomenclature is just underthinking while overthinking, making method names more confusing to people trying to use them.
Like Every function you’ll ever use, read about what it does before using it, no need to name it “Try”, if you want consistency maybe also name it Request? you’re requiring a response from the Engine about whether or not you can start recording, so, why “Try” if the method is supposed to deliver you an answer, wouldn’t it just be Request? Try assumes that it being denied is a failure, instead of a valid, expected response, Just like Publish methods which can (or could, limits changed recently and i haven’t read about it) be denied for going over the limit…
I would have to disagree with this strongly. The method name is succinct - you call the method to request the engine to try to record a change history, and if it’s successful you get back a recording object that you pass back to FinishRecording.
TryBeginRecording is readable and understandable in plain English. Method names and variables should be clear what they do from the outset. I should not need to read documentation to get a summary understanding of what a method does, only when I want to know implementation details.
I do get the whole argument about semantics but they don’t hold water here. Async methods are named that way because of their yielding behaviour, not their potential to fail. When you call those methods they are expected to work, and the errors they throw are because of exceptions (issues that some party needs to fix). TryBeginRecording does not error because a nil return is not an error. Errors should only be thrown for exceptions - they’re called exceptions for a reason.
Adjusting the naming convention or throwing an error when unnecessary adds friction to the developer experience for no benefit. If the engine can control the source of error, a nil (or honestly other distinct symbol) is appropriate enough. I only want to perform error handling in cases where I expect an error such as making requests to third party servers or spaces I have no control over.
You misunderstood me, it’s not the “Async” which is expected to fail, it’s the “Request”, which follows the same logic.
My point is that the Terminology is inconsistent, and consistency is good in programming, you should be able to guess function names based on what they do, and choosing a different word every time you do a new method based on the same category goes against that principle.
And i’m not arguing for change, what’s done is done, i’m arguing that it was a bad choice and that should be considered in the future.
Shorter, more consistent names are better, specially for common methods. (i couldn’t tell how annoyed i get at the length of some method names which i use all the time like FindFirstChild, i could only dream of having both this name and an abbreviated “codename” for the method like :FFC().
Does this essentially mean you will blow up the output with warnings for all users who have installed and is running a plugin which doesn’t incorporate the new methods just like you did with PhysicsService:CreateCollisionGroup() → :RegisterCollisionGroup()?
Because that would be a very bad idea. Rather display a warning in studio’s script analysis to encourage incorporating the new methods in new work, instead of pushing plugin developers to update all their past plugins solely for this purpose.
From some base testing I’ve done with the ChangeHistoryService changes on auroraSuite, things can really break if for whatever reason Studio runs into an error on the script that is recording.
Edit: It’s really my fault though, I’m pretty blind. See the response from Roblox staff below this.
As an example, I received ‘unable to cast null to token’ on the finish recording function with the script below, and even if I relaunch my plugin by saving the plugin files to local plugin, functions with the script that ran into an error just don’t run. I have to close a place and reopen it to be able to use the specific script again.
Hoping this sounds descriptive enough, fingers crossed.
script.Parent.MouseButton1Click:Connect(function()
local st = script.Parent.Parent.StartBox
local samt = script.Parent.Parent.IntervalBox
local chs = game:GetService("ChangeHistoryService")
local tag = game:GetService("CollectionService")
if st.Text == "" then
error("auroraSuite: Unable to create day/night cycle due to not having any integer values in some textboxes.")
elseif samt.Text == "" then
error("auroraSuite: Unable to create day/night cycle due to not having any integer values in some textboxes.")
elseif game.ServerScriptService:FindFirstChild("DayAndNightScript") then
warn("auroraSuite: A day/night cycle script was already found, and we're unable to edit the existing one. Try deleting the existing one first.")
else
local record = chs:TryBeginRecording("auroraSuite: Day/Night Cycle")
if record then
local sc = Instance.new("Script")
sc.Parent = game.ServerScriptService
sc.Name = "DayAndNightScript"
sc.Source = 'local minutesAfterMidnight = ' .. st.Text ..
'\n while true do \n' ..
'game.Lighting:SetMinutesAfterMidnight(minutesAfterMidnight) \n' ..
'minutesAfterMidnight = minutesAfterMidnight + 1 \n' ..
'task.wait(' .. samt.Text .. ') \n' ..
'end'
tag:AddTag(sc, "auroraSuite-generated")
if plugin:GetSetting("DebugMode") == true then
print("auroraSuite: Made day and night cycle successfully in game.ServerScriptService.")
plugin:OpenScript(sc)
end
chs:FinishRecording(record, Enum.FinishRecordingOperation)
end
end
end)
20:35:42.439 Unable to cast null to token - Edit - Script:32
20:35:42.439 Stack Begin - Studio
20:35:42.439 Script 'user_aurorasuite.rbxmx.aurorasuite.UI.ScrollingFrame.DayNightCycleOpen.TextButton.Script', Line 32 - Studio - Script:32
20:35:42.439 Stack End - Studio
I can’t see Roblox trying to push plugin developers to be honest, I’m still fed up of Roblox not addressing anything that I’ve ever said about the Plugin Marketplace. Meanwhile, other creators are literally talking on the phone with Roblox about the Marketplace. I’m part of the same beta group as them as well.
Looking at your code and trying to understand what went wrong / what can be done better, perhaps you want to make some different choices to get more feedback / control error usage. Also, Enum.FinishRecordingOperation is then name of the enum, not a value like Append, Cancel, or Commit – the FinishRecording method takes one of the values.
Here is the style of what I’m thinking you want your code to look like:
-- If you encapsulate your callback in a named function the name will appear
-- in errors in the output. I find it a little cleaner.
local function onMouseClick()
if --[[error condition 1]] then
error("Error 1 description")
elseif --[[error condition 2]] then
error("Error 2 description")
elseif --[[warning condition 3]] then
warn("Warning")
return -- avoid "arrow anti-pattern" - don't put rest of method in else.
end
local record = chs:TryBeginRecording(...)
if not record then
warn("Can not record.") -- some feedback here to know what happened
return
end
local success, result = pcall(function()
--[[
Operations here that may fail with an error produce success = false
]]
end)
-- Regardless of how the pcall above completes, you will be able to finish
-- the recording here.
chs:FinishRecording(record, if success then
Enum.FinishRecordingOperation.Commit else
Enum.FinishRecordingOperation.Cancel)
-- As an added bonus, you can report the error string.
if not success then
warn("Failed to complete recording. Error: " .. result)
end
end
script.Parent1.MouseButton1Click:Connect(onMouseClick)
Don’t panic. We expect this migration will take a long time, I’m guessing at least 6 months, and we don’t want to warn plugin users, but we do want to notify plugin authors - whether that be via warn("Making changes without using ChangeHistoryService:TryBeginRecording() is deprecated -- see https://devforum.roblox.com/t/recordings-in-change-history-service/2512500") and something similar on SetWaypoint – we intend to only show those when the plugin is local or the author of the plugin is the current user.
Definitely a good update if you ask me. I always get the “ChangeHistoryService is currently running” error message popup whenever I try to undo a large amount of changes. I hope this will eliminate the error and overall be more efficient. Excited to see the future updates!
There are no such restrictions. We designed the API to handle cases where your plugin may take as long as necessary to complete the recording, and it doesn’t matter what thread of execution - it only matters that the API is called from the same script context (Studio, plugin, or command bar). There is a 1:1 mapping from script context to allowed recording.
Will their be ways where you can delete some older versions of your game in the version history section?
Just to clear it up and if you need to back up you dont accidentally pick an older one with possibly a bad bug?
example - you have an older version of your game and you updated it because there was a bad bug. And you now have the older version in your version history. Will you be able to delete?
it would just help make the place so much cleaner!
ChangeHistoryService only works within a single session of Studio – even across multiple users on TeamCreate - each user’s change history contains only their own changes made by that user in that session.
Technically, you can archive a whole place (notably from the website) - maybe if you have a main game place and a bunch of related sub-places you could archive one of them and replace it with a new place. However, as far as I can tell, you can’t delete / archive a version from the history of a place.
If you are worried about bugs with Scripts and Models and other contents built into a place, may I suggest that you create Packages which you can manage across places and then you may enable the auto-update property on the PackageLink instance to pick up latest changes in Studio before re-publishing your affected places. For more information, you may find Packages documentation here.
Sorry for the small bump, but I am having some issues with this.
Is there a way to make multiple recordings at once? The problem is that my plugin could potentially run multiple yielding functions at once, but only 1 recording can be made at a time.
Hey! I wanted to drop this here in case any team members working on these changes is still looking. I was wondering whether empty recordings can be relied upon as a way of inserting custom non-DataModel undo actions?
Supporting this officially would be better (Non-DataModel undo/redo), but I’m just wondering how this interacts with e.g. Team Create.
After testing this, it doesn’t work as expected. Undoing after my plugin makes a recording undoes to earlier than the start of recording, it undoes to the end of the previous action.
