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

Track patch sources, make the plugin ignore its own changes #699

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion plugin/src/ApiContext.lua
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ end
local ApiContext = {}
ApiContext.__index = ApiContext

function ApiContext.new(baseUrl)
function ApiContext.new(baseUrl,clientId)
assert(type(baseUrl) == "string", "baseUrl must be a string")

local self = {
Expand All @@ -94,6 +94,7 @@ function ApiContext.new(baseUrl)
__messageCursor = -1,
__connected = true,
__activeRequests = {},
__clientId = clientId
}

return setmetatable(self, ApiContext)
Expand Down Expand Up @@ -189,6 +190,7 @@ function ApiContext:write(patch)

local body = {
sessionId = self.__sessionId,
clientId = self.__clientId,
removed = patch.removed,
updated = updated,
added = added,
Expand Down
5 changes: 4 additions & 1 deletion plugin/src/App/init.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")
local HttpSevice = game:GetService("HttpService")

local Rojo = script:FindFirstAncestor("Rojo")
local Plugin = Rojo.Plugin
Expand Down Expand Up @@ -211,12 +212,14 @@ function App:startSession()
local baseUrl = if string.find(host, "^https?://")
then string.format("%s:%s", host, port)
else string.format("http://%s:%s", host, port)
local apiContext = ApiContext.new(baseUrl)
local clientId = HttpSevice:GenerateGUID(false)
local apiContext = ApiContext.new(baseUrl, clientId)

local serveSession = ServeSession.new({
apiContext = apiContext,
openScriptsExternally = sessionOptions.openScriptsExternally,
twoWaySync = sessionOptions.twoWaySync,
clientId = clientId
})

serveSession:onPatchApplied(function(patch, _unapplied)
Expand Down
6 changes: 6 additions & 0 deletions plugin/src/ServeSession.lua
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ local validateServeOptions = t.strictInterface({
apiContext = t.table,
openScriptsExternally = t.boolean,
twoWaySync = t.boolean,
clientId = t.string
})

function ServeSession.new(options)
Expand Down Expand Up @@ -99,6 +100,7 @@ function ServeSession.new(options)
__statusChangedCallback = nil,
__patchAppliedCallback = nil,
__connections = connections,
__clientId = options.clientId
}

setmetatable(self, ServeSession)
Expand Down Expand Up @@ -288,6 +290,10 @@ function ServeSession:__mainSyncLoop()
Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages)

for _, message in ipairs(messages) do
if message.clientId == self.__clientId then
continue
end

local unappliedPatch = self.__reconciler:applyPatch(message)

if not PatchSet.isEmpty(unappliedPatch) then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ messages:
Name: my-new-folder
Parent: id-2
Properties: {}
clientId: ~
removed: []
updated: []
sessionId: id-1

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
source: tests/tests/serve.rs
expression: "subscribe_response.intern_and_redact(&mut redactions, ())"

---
messageCursor: 1
messages:
- added: {}
clientId: ~
removed: []
updated:
- changedClassName: ~
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: tests/tests/serve.rs
expression: "subscribe_response.intern_and_redact(&mut redactions, ())"

---
messageCursor: 1
messages:
Expand All @@ -15,6 +14,7 @@ messages:
Name: test
Parent: id-2
Properties: {}
clientId: ~
removed: []
updated: []
sessionId: id-1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: tests/tests/serve.rs
expression: "subscribe_response.intern_and_redact(&mut redactions, ())"

---
messageCursor: 1
messages:
Expand Down Expand Up @@ -135,6 +134,7 @@ messages:
Properties:
Value:
String: "File #5"
clientId: ~
removed: []
updated: []
sessionId: id-1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ expression: "subscribe_response.intern_and_redact(&mut redactions, ())"
messageCursor: 1
messages:
- added: {}
clientId: ~
removed:
- id-3
updated: []
sessionId: id-1

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
source: tests/tests/serve.rs
expression: "subscribe_response.intern_and_redact(&mut redactions, ())"

---
messageCursor: 1
messages:
- added: {}
clientId: ~
removed: []
updated:
- changedClassName: ~
Expand Down
6 changes: 5 additions & 1 deletion src/snapshot/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use super::{InstanceMetadata, InstanceSnapshot};
/// conflict!
#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
pub struct PatchSet {
pub client_id: Option<String>,
pub removed_instances: Vec<Ref>,
pub added_instances: Vec<PatchAdd>,
pub updated_instances: Vec<PatchUpdate>,
Expand All @@ -22,6 +23,7 @@ pub struct PatchSet {
impl PatchSet {
pub fn new() -> Self {
PatchSet {
client_id: None,
removed_instances: Vec::new(),
added_instances: Vec::new(),
updated_instances: Vec::new(),
Expand Down Expand Up @@ -63,14 +65,16 @@ pub struct PatchUpdate {
// current values in all fields.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct AppliedPatchSet {
pub client_id: Option<String>,
pub removed: Vec<Ref>,
pub added: Vec<Ref>,
pub updated: Vec<AppliedPatchUpdate>,
}

impl AppliedPatchSet {
pub fn new() -> Self {
pub fn new(client_id: Option<String>) -> Self {
AppliedPatchSet {
client_id: client_id,
removed: Vec::new(),
added: Vec::new(),
updated: Vec::new(),
Expand Down
12 changes: 11 additions & 1 deletion src/snapshot/patch_apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use super::{
/// tree in sync with Rojo's.
#[profiling::function]
pub fn apply_patch_set(tree: &mut RojoTree, patch_set: PatchSet) -> AppliedPatchSet {
let mut context = PatchApplyContext::default();
let mut context = PatchApplyContext::new(patch_set.client_id);

{
profiling::scope!("removals");
Expand Down Expand Up @@ -76,6 +76,16 @@ struct PatchApplyContext {
applied_patch_set: AppliedPatchSet,
}

impl PatchApplyContext {
fn new(client_id: Option<String>) -> Self {
Self {
snapshot_id_to_instance_id: HashMap::new(),
has_refs_to_rewrite: HashSet::new(),
applied_patch_set: AppliedPatchSet::new(client_id),
}
}
}

/// Finalize this patch application, consuming the context, applying any
/// deferred property updates, and returning the finally applied patch set.
///
Expand Down
2 changes: 2 additions & 0 deletions src/snapshot/patch_compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ mod test {
let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);

let expected_patch_set = PatchSet {
client_id: None,
updated_instances: vec![PatchUpdate {
id: root_id,
changed_name: None,
Expand Down Expand Up @@ -310,6 +311,7 @@ mod test {
let patch_set = compute_patch_set(Some(snapshot), &tree, root_id);

let expected_patch_set = PatchSet {
client_id: None,
added_instances: vec![PatchAdd {
parent_id: root_id,
instance: InstanceSnapshot {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
source: src/snapshot/tests/apply.rs
expression: applied_patch_value

---
client_id: ~
removed: []
added: []
updated:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
source: src/snapshot/tests/apply.rs
expression: applied_patch_value
---
client_id: ~
removed: []
added: []
updated:
Expand All @@ -11,3 +12,4 @@ updated:
changed_properties:
Foo: ~
changed_metadata: ~

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
source: src/snapshot/tests/apply.rs
expression: applied_patch_value
---
client_id: ~
removed: []
added: []
updated:
Expand All @@ -10,3 +11,4 @@ updated:
changed_class_name: Folder
changed_properties: {}
changed_metadata: ~

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
source: src/snapshot/tests/compute.rs
expression: patch_value
---
client_id: ~
removed_instances: []
added_instances:
- parent_id: id-1
Expand All @@ -16,3 +17,4 @@ added_instances:
properties: {}
children: []
updated_instances: []

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
source: src/snapshot/tests/compute.rs
expression: patch_value
---
client_id: ~
removed_instances:
- id-2
added_instances: []
updated_instances: []

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
source: src/snapshot/tests/compute.rs
expression: patch_value
---
client_id: ~
removed_instances: []
added_instances: []
updated_instances:
Expand All @@ -11,3 +12,4 @@ updated_instances:
changed_properties:
Foo: ~
changed_metadata: ~

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
source: src/snapshot/tests/compute.rs
expression: patch_value
---
client_id: ~
removed_instances: []
added_instances: []
updated_instances:
Expand All @@ -10,3 +11,4 @@ updated_instances:
changed_class_name: Folder
changed_properties: {}
changed_metadata: ~

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
source: src/snapshot/tests/compute.rs
expression: patch_value

---
client_id: ~
removed_instances: []
added_instances: []
updated_instances:
Expand Down
1 change: 1 addition & 0 deletions src/web/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl ApiService {

tree_mutation_sender
.send(PatchSet {
client_id: request.client_id,
removed_instances: Vec::new(),
added_instances: Vec::new(),
updated_instances,
Expand Down
3 changes: 3 additions & 0 deletions src/web/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub const PROTOCOL_VERSION: u64 = 4;
#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct SubscribeMessage<'a> {
pub client_id: Option<String>,
pub removed: Vec<Ref>,
pub added: HashMap<Ref, Instance<'a>>,
pub updated: Vec<InstanceUpdate>,
Expand Down Expand Up @@ -72,6 +73,7 @@ impl<'a> SubscribeMessage<'a> {
.collect();

Self {
client_id: patch.client_id,
removed,
added,
updated,
Expand Down Expand Up @@ -179,6 +181,7 @@ pub struct ReadResponse<'a> {
#[serde(rename_all = "camelCase")]
pub struct WriteRequest {
pub session_id: SessionId,
pub client_id: Option<String>,
pub removed: Vec<Ref>,

#[serde(default)]
Expand Down