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

WIP: Refactor of cloud game overlord and overworker #70

Merged
merged 8 commits into from Sep 11, 2019

Conversation

@sadlil
Copy link
Contributor

commented Aug 21, 2019

Work In Progress.

Fixes #68

@sadlil sadlil force-pushed the sadlil:master branch from 6a2c080 to df75987 Aug 21, 2019

@giongto35

This comment has been minimized.

Copy link
Owner

commented Aug 21, 2019

@_@ wow, it's a massive change. When it finishes, it gonna be awesome. (y)

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2019

@giongto35 Hope so. I am still doing the changes. Hopefully, you can start to review some part from now. :) Thanks, man.

@giongto35

This comment has been minimized.

Copy link
Owner

commented Aug 21, 2019

Yes I'm checking :P

pkg/worker/handlers.go Outdated Show resolved Hide resolved
@giongto35

This comment has been minimized.

Copy link
Owner

commented Aug 22, 2019

I have a new update on encoder. You may need to introduce new files.
https://github.com/giongto35/cloud-game/pull/70/files

added files:

encoder/type.go
h264encoder/encoder.go

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

@giongto35 Go ahead and merge, please. I will fix the conflicts in my branch. :)

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2019

@giongto35 I have fixed all the merge issues. But while running My game is failing as my nanoarc.go:L345 is failing to find the game. I have moved the emulators to assets/emulator/libretro/cores directory as I think this is more appropriate palace. Can help me with this. What is this url represents? Is it relative to something? Or I need to put the .so files in the same directory?

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

To me the problem looks like a Mac issue. Did you happen to encounter this problem before? Let me know. @giongto35

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

@giongto35 I have completed the first version of refactoring the code. Apart from the issue with C.dlopen everything else works fine. Please have a look. Help fix the error and if its looks okay - lets merge it. For reference, we need to fix the following in future iterations for refactoring

  1. Use flags embedded in binary instead of global flags
  2. Few more code refactoring inside overlord and worker codes
  3. Rewrite Readme
  4. Organize and fix docker files and builds
  5. Get rid of pkg/config and reorganize pkg/util
  6. Write E2e tests and move main_test.go to tests/e2e using ginkgo - example - https://github.com/appscode/voyager/blob/master/test/e2e/e2e.go
@giongto35

This comment has been minimized.

Copy link
Owner

commented Sep 11, 2019

Sorry @sadlil, I have a problem of github notification, so I forgot to check. :D
This is massive and necessary. I 'm not aware of the problem with Mac because I develop it on Ubuntu. You re so good figuring it out.
And I prioritize your merge, Hopefully to get it merged today or tomorrow. Will not spoil any conflict :p

@sadlil

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

@giongto35 It okay. No worries. Take your time to review and please have a short testing session to check everything works well with this patch.

@giongto35

This comment has been minimized.

Copy link
Owner

commented Sep 11, 2019

FYI, vendor removed all C files related to x264 :( . I need to use https://github.com/goware/modvendor to get it back.
BTW, I saw you using vendor but don't have any script based on vendor. I think vendor can only help when we build with -mod=vendor

@giongto35 giongto35 merged commit 8769e0c into giongto35:master Sep 11, 2019

@giongto35

This comment has been minimized.

Copy link
Owner

commented Sep 11, 2019

I love the new log <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.