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

How to get source id for item #321

Open
matthijskooijman opened this issue Oct 1, 2020 · 5 comments · May be fixed by #325
Open

How to get source id for item #321

matthijskooijman opened this issue Oct 1, 2020 · 5 comments · May be fixed by #325
Assignees
Milestone

Comments

@matthijskooijman
Copy link

I'm looking to identify which items are linked to the same source (more specifically, when iterating over all items, only process one item for each linked source and skipping subsequent ways). An obvious way to do so would be to get the id of the source linked to an item and see if that id is the same.

It think I can do this using Item._srcId, but that's of course an internal property. There does not seem to be an Item.getSourceId() or so, would that be worth adding?

I could of course do getSource().getId(), but that involves several async requests (since getSource() requests config rather than using this._xmlparams which I suspect is the same, and getId() for a source requests prop:srcid rather than returning just this._srcId, for reasons I don't follow) and constructs a new object, which should not really be needed.

@dcefram
Copy link
Contributor

dcefram commented Oct 5, 2020

Hi, sorry for the late response. First off, I'll just give some sort of a direction on what you can do to achieve what you intended to do, assuming that you just want to simply skip the linked items if they were already "processed" at least once:

  • get all sources and iterate through each of the source (source.getAllSources())
  • for each source, call getItemList(), and just get the first item in that list. Process that item.

Pseudocode:

call source.getAllSources() and iterate it:
  for each source, call getItemList():
    get the first item of the list, and do whatever you need to do with the item

As for why does xjs always request config rather than using the cached value, it's more to do with the fact that the value of the "cached" _xmlparams can easily be outdated and inaccurate since you can (or another plugin could) modify your presentation. Considering that fact, it'll be a troublesome bug to track if in case you're trying to manipulate an item/source that does not exist in your presentation anymore just because xjs was using the cached/internal XML instead of requesting the latest one.

Edit: as for why does xjs always requests prop:srcid rather than just using this._srcId, this is to handle scenarios where the user unlinks a linked item. This could cause the srcid to change internally, and thus would again cause bugs if xjs is to reference the cached, now-outdated srcid.

Hope that helps.

@matthijskooijman
Copy link
Author

Thanks for your response. What you propose now is pretty much exactly what we're doing now (except iterating sources per scene rather than all at once), but it's too slow. We have a project where most sources have a dozen or so linked items, and we've found that getItemList() is really slow here having to lookup and create Item objects of which we'll only use one.

We have a project with around 500 items and maybe 50-100 sources, and iterating them all took minutes. We then realized that we didn't actually need getItemList() for what we did and we ended up using the source directly, which made things a order of magnitude better. However, in order to speed it up a bit more, I want to just iterate items, rather than sources, since AFAIU all operations happen on items internally anyway, so operating on sources is slower because it has to doublecheck that the item used internally still exists.

As for why does xjs always request config rather than using the cached value, it's more to do with the fact that the value of the "cached" _xmlparams can easily be outdated and inaccurate since you can (or another plugin could) modify your presentation.

True, but the same thing applies to the Item that you would call getSource() on, so in the face of parallel updates, you'll probably have to take additional measures anyway maybe. But I see the point, just making sure that whenever a new object is created, it is made up-to-date, does make sense.

Edit: as for why does xjs always requests prop:srcid rather than just using this._srcId, this is to handle scenarios where the user unlinks a linked item. This could cause the srcid to change internally, and thus would again cause bugs if xjs is to reference the cached, now-outdated srcid.

But when an item is unlinked, wouldn't that just duplicate the source, so that he old id is still valid but a new one is added? And wouldn't it be more correct for the Source object to return its own original id? I would say that a Source object actually corresponds to some specific source, independent of the item that happens to be associated with that source to do lookups?

More specifically, consider:

  • Two items I1 and I2, both referring to a single source S1
  • I have a reference to I2, call getSource() on it, and get a reference to S1. When I call getId() on that, I get e.g. "S1".
  • Now I unlink I2, which I would expect creates a new source S2 (so now I1 -> S1, I2 -> S2).
  • If I now call getId() on the source I had before, I think it would return "S2".

This seems wrong: I had a reference to a source, which is suddenly changed to another source, which seems wrong to me. Unless you interpret a Source object as "the source for a given item", but that seems unexpected to me (also, when the given item is deleted, I think it is replaced by an arbitrary other item by wrapGet() or so, which does not make too much sense when it would be the source for a given item).

@dcefram
Copy link
Contributor

dcefram commented Oct 8, 2020

We then realized that we didn’t actually need getItemList() for what we did and we ended up using the source directly, which made things a order of magnitude better. However, in order to speed it up a bit more, I want to just iterate items, rather than sources, since AFAIU all operations happen on items internally anyway, so operating on sources is slower because it has to doublecheck that the item used internally still exists.

In XJS, when you’re trying to get or set a property of an item, both the core/items/item.ts (and all other item variants) and core/source/source.ts (and all other source variants) would make use of internal/item.ts’ get, wrapGet, set, wrapSet methods. With that in mind, the difference between using the item class and source class is minimal in terms of the level of indirection since they both use the same internal static methods.

This seems wrong: I had a reference to a source, which is suddenly changed to another source, which seems wrong to me.

I would have to agree with you on this one. A source shouldn’t change to another source, and the only way for a source to be “invalid” is if it was deleted. But what you said in the second part is true, which is why it currently behaves this way:

Unless you interpret a Source object as “the source for a given item”

The reason why XJS was designed in such a way rather than the other way around is mainly due to the fact that XJS 2 was created and released before XSplit had a concept of "sources" and "items" (ie. before the "linked items" feature came around). The fact that existing plugins that use XJS already treats the items as the base object, the whole sources concept was then treated as just another property to get from an item.

Just to give an example of an issue that we tried to avoid:

  • item1 in scene1
  • user copies item1 to scene2 as a linked item (we'll refer this as item2)
  • an extension plugin iterates items in scene2, and would interact with them
  • extension plugin gets item2, and gets its source. Let's call this as source1
  • user unlinks item2
  • user renames item2
  • extension plugin then proceeded to call source1.getName()
  • extension plugin would get the name of item1 in scene1, even though the intention here is that it would use item2 in scene2

We could've made the sources as the base of all items in XJS, but it could be treated as a breaking change... and that would mean releasing a major version to denote that whatever concept we had in XJS2 would most likely not apply in the new version.

@matthijskooijman
Copy link
Author

matthijskooijman commented Oct 8, 2020

the difference between using the item class and source class is minimal in terms of the level of indirection since they both use the same internal static methods.

Yeah exactly, though I still expect operating on sources to be (somewhat) slower because of the extra code in wrapGet over get. Might be negligable, though.

The reason why XJS was designed in such a way rather than the other way around is mainly due to the fact that XJS 2 was created and released before XSplit had a concept of "sources" and "items" (ie. before the "linked items" feature came around).

Yeah, I had guessed as much from looking at the internals. Doesn't make the code easier, but I can see how this affects compatibility indeed.

Just to give an example of an issue that we tried to avoid:

I think that's pretty much the same as the example I gave, though you talk about getting the name rather than the source id. And I guess the name of a source is actually the name of an item, so different items have different names even when they share the same source. So what makes it weird is that you can access Item properties through a Source object, which does not make sense conceptually, but is needed for compatibility because what's now an Item used to be called Source before linked sources. I can see the problem, yeah :-)

Anyway, I think I know enough to move forward here. The core of the issue remains open, though: Should an Item get a getSourceId() method, that essentially just shortcuts getSource().getId() (without the overhead of constructing a Source object and extra requests, by just returning _srcId directly, or maybe just doing one request to ensure the latest value)?

@dcefram
Copy link
Contributor

dcefram commented Oct 9, 2020

Yeah, adding that method does make sense and shouldn't be hard to do.

Edit: @SML-MeSo is there an active milestone that we can sneak this changes to?

@SML-MeSo SML-MeSo added this to the 2.11.0 milestone Oct 12, 2020
@dcefram dcefram self-assigned this Oct 23, 2020
@dcefram dcefram linked a pull request Oct 23, 2020 that will close this issue
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 a pull request may close this issue.

3 participants