Flexible extension to RFrames

[codejaeger]

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

• Did I update CHANGELOG.md with whatever changes/features I added with this PR?
• Did I make sure to only change the part of the file where I introduced a new change/feature?
• Did I cover all corner cases to be close to 100% test coverage (if applicable)?
• Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
• Did I properly add test dependencies to the test directory (if applicable)?
• Did I check relevant tutorials that may be affected by changes in this PR?
• Did I clearly articulate why this PR was made the way it was and how it was made?

Link to relevant issue(s)
Closes #331

How did you address these issues with this PR? What methods did you use?
As discussed in the issue there were 2 approaches to make RFrames more flexible. One was to supply additional flags to accommodate the different cases. Other is to let the user specify the starting reference and (optionally) the end frame.
The latter allows a wide range of options like specifying the frame range based on a previously created object.
Any suggestions @Wikunia @TheCedarPrince @Sov-trotter ?

[codecov]

Codecov Report

Merging #334 (a219185) into master (4241ea1) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #334      +/-   ##
==========================================
+ Coverage   96.17%   96.29%   +0.12%     
==========================================
  Files          21       21              
  Lines        1151     1189      +38     
==========================================
+ Hits         1107     1145      +38     
  Misses         44       44              

Impacted Files	Coverage Δ
src/Javis.jl	95.23% <ø> (ø)
src/structs/Frames.jl	100.00% <100.00%> (ø)
src/structs/RFrames.jl	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4241ea1...a219185. Read the comment docs.

[Wikunia]

I find this a little hard to read. What exactly is the meaning of default_last() when combined with prev_start here?

[Wikunia]

Thanks for opening this PR @codejaeger
I find some of the variants currently a bit hard to read. Have we thought of the following before?
Another idea might be to use Frames(prev_start()+5, prev_start()+10) to provide a start and end frame. Or maybe easier
Frames(prev_start()+5, 10) and the second parameter is always the length. I would more or less leave RFrames as it is then and just have a generic approach with Frames where the user can do whatever. Default to length of the object action seems reasonable to me but using Frames(prev_start(); stop=100) would make it straightforward I think.

[Wikunia]

I think a good way to have a lot of flexibility would be to use:

macro Frames(start, len)
         return :(()->$start:$len)
end

and then with simple functions like this:

prev_start() = CURRENT_OBJECT[1].frames.frames[1]

This would be possible:

@Frames(prev_start(), 10)

Javis just needs to call the anonymous macro function then to get the frames.
We have to check how we get something like prev_start work with actions as well and have maybe an parent_start to get the object instead of the previous action as an example.

Additionally maybe we can provide a macro with start, stop instead of start, len if that is desired.

[Wikunia]

@codejaeger you can let me know here if you want to continue working on it. If not I might implement this myself in the next couple of days.

[codejaeger]

Hey @Wikunia , here are my views and thoughts on the approach. Your idea seems a much more readable way to do the job than what the PR proposes. I have just a few doubts on the approach.

1. Is the usage going to be similar to Object(@Frames(prev_start(), 10), (args...) -> dummy())?
2. The stop parameter seems a global frame number to me. The way frames are stored in actions is relative to the Object. So this parameter does not seem very useful for Actions but it is important for Objects anyways.
3. The reason I was able to adjust parent_start to actions was because I had the type of the elem the frame belonged to available from inside the get_frames method.
4. I tried this code and right now it seems the CURRENT_OBJECT array is not updated at the end of Object creation. Is that a bug? I don't see it being used anywhere so it might have gone undetected.
5. Once the issue in the previous point is fixed, I think it would be easy to extend prev_start for actions since the actions are also stored in an array in the current object. Although, we might need a different version of prev_start e.g. prev_action_start. What do you think?

[Wikunia]

1. Yes it will be that syntax
2. We might want to have it relative to either the global one or the parent object. Similar to how the frames work now for the action
3. I'll need to look into this more
4. The current object only changes inside render as it should be independent of when it was created.
5. I think it should work well with actions and should just be relative to the parent object.

[Wikunia]

We might replace this by #338

[Wikunia]

Can we close this @codejaeger ?

[codejaeger]

Yes, sure @Wikunia .