speedcontrol / nodecg-speedcontrol

NodeCG bundle to help facilitate the running of speedrunning marathons, including overlays.
MIT License
44 stars 34 forks source link

Add feature importing schedule from Oengus.io #71

Closed cma2819 closed 4 years ago

cma2819 commented 4 years ago

Oengus.io is recently used well. We often manage schedule on it without horaro. Oengus has the API endpoint to get schedule. It makes easy to import schedule from NodeCG.

This is useful feature for Oengus user, I wish this would be better for new feature on speedcontrol.

I'm new to contribute OSS. Feel free to advise me :)

zoton2 commented 4 years ago

Looks good from what I've skimmed of it, currently at an event and will check it out more when I get home. It would be nice to have both schedule imports in the same dashboard panel and just make it so you choose which you want to use in the configuration, although not sure if that would be a good idea for v2.

zoton2 commented 4 years ago

Apologies for not looking at this yet, been pretty busy. If you've been using it yourself, has it been working OK?

cma2819 commented 4 years ago

No problem. I'm using this for making bundle working with speedcontrol, and this is working well.

I haven't made in same dashboard panel with Horaro schedule import. This is separated from it.

zoton2 commented 4 years ago

I'm actually helping with a marathon next weekend that is using Oengus so may test it for that myself.

cma2819 commented 4 years ago

Thanks. If you would get any problems with this, I'll fix it.

zoton2 commented 4 years ago

I tested this briefly myself last night and the only issue I saw was when I did the initial import over my old runs it didn't appear to update, but I need to try again to see if that's an actual issue or something I messed up. Besides that, someone else tried it on 2 machines too and the basics seem fine to them too. Some documentation will need adding but I can do that if you're not comfortable with English enough to do that.

I see in your own master you added some parts that change the internal IDs to the ones supplied by Oengus, I'm not sure if you intend to put those in this PR too but I would advise against it as I have another idea for external IDs that I want to implement, so I think keeping the internal ID the way it is is for the best right now.

cma2819 commented 4 years ago

Thanks for your feedback. I haven't seen the issue to failed initial importing. Perhaps I should test with the same marathon and check repeatability of it. For documentation about this, I have never written the documentation in English as OSS. I would appreciate it if you could do documentation about this.

And I'll be sorry about the changing the internal ID when this would be used. I set Oengus's user ID and category ID in speedcontrol's replicant because I want to make relationship from Oengus's data in another bundle to speedcontrol's replicant data. But it's definitely wrong way to use internal ID. I should add new property in schema of speedcontrol's replicant and I should use it for memorizing Oengus's ID.

I think it's better to intend add properties on Oengus in this PR. I'm going to fix to make

RunData.customData.oengusCategoryID

for Oengus's category ID and add

players.*.social.oengus

for Oengus's user ID. If you would have another idea, I want to get your advice.

Example of Oengus's schedule is here: https://oengus.io/api/marathon/rta1kagawa/schedule

zoton2 commented 4 years ago

My other idea was to just add an "external ID" field to the relevant places, without being specific about where it's from, as I assume there should never be multiple. See #72, which my idea was to implement that for ESA Winter but I didn't get the time. We have our own tool that exports IDs into a Horaro column alongside the scheduled runs, and my main reason for the external IDs in that case was to make re-imports better if the schedule has changed.

Although for the user ID's, maybe we should add an Oengus to the social object; I assume those IDs are the same site wide as they log in themselves?

I'd suggest keeping anything programmed internally out of the customData object, just as I programmed it with the intention of it only to be used for the user's custom data.

I can add some of this stuff myself if need be, you don't have to do everything. 😅

cma2819 commented 4 years ago

Okay, I leave staff of external ID support. This PR only includes importing schedule from Oengus feature.

Thanks for advice :)

zoton2 commented 4 years ago

Just been doing some minor tweaks and I realised that the oengusAPIData schema/replicant is currently unused it seems? Maybe it could be removed for now if not needed.

cma2819 commented 4 years ago

It’s unused replicant’s schema, it’s could be removed now. I don’t remember why it’s defined. Sorry for left unused file in PR😔

zoton2 commented 4 years ago

Made a few tweaks, will merge this now and do anything else I want to do in this repo.

cma2819 commented 4 years ago

Thanks for merging.