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

Remove hifi screenshare (updated) #757

Closed

Conversation

daleglass
Copy link
Contributor

@daleglass daleglass commented Dec 6, 2023

PR #435, rebased.

Original text below:

Cherry-picked and updated from Tivoli dd5b6ea6ee5597a06603e16509640e7ed18106bb

As much of a hoarder as I am, I really don't think we will ever make use of this screen share app. The way it is currently supposed to work isn't any better than a regular web browser, and I don't think we will ever modify WebRTC to make it any better than a web browser.

This includes a placeholder to not break protocol.
Tested on Linux.

Closes #435

@daleglass daleglass marked this pull request as draft December 6, 2023 12:27
@daleglass
Copy link
Contributor Author

@HifiExperiments I incorporated your review comments into the last commit, please take a look!

@daleglass daleglass added needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested labels Dec 6, 2023
@daleglass daleglass marked this pull request as ready for review December 6, 2023 12:36
@ksuprynowicz
Copy link
Member

Windows build is failing for me with an unrecognized symbol:

image.lib(OpenEXRReader.obj) : error LNK2001: nierozpoznany symbol zewnętrzny imath_half_to_float_table [C:\Users\ksupr\source\overte\overte_v8\cmake-build-relwithdebinfo-visual-studio\plugins\openvr\openvr.vcxproj]
C:\Users\ksupr\source\overte\overte_v8\cmake-build-relwithdebinfo-visual-studio\plugins\openvr\RelWithDebInfo\openvr.dll : fatal error LNK1120: liczba nierozpoznanych elementów zewnętrznych: 1 

@daleglass
Copy link
Contributor Author

Windows build is failing for me with an unrecognized symbol:

image.lib(OpenEXRReader.obj) : error LNK2001: nierozpoznany symbol zewnętrzny imath_half_to_float_table [C:\Users\ksupr\source\overte\overte_v8\cmake-build-relwithdebinfo-visual-studio\plugins\openvr\openvr.vcxproj]
C:\Users\ksupr\source\overte\overte_v8\cmake-build-relwithdebinfo-visual-studio\plugins\openvr\RelWithDebInfo\openvr.dll : fatal error LNK1120: liczba nierozpoznanych elementów zewnętrznych: 1 

That was fixed in PR #756

Just rebased it on top of the latest code, that should do it. Sorry about that ^_^;;

@ksuprynowicz
Copy link
Member

Interface builds now :)

Copy link
Member

@HifiExperiments HifiExperiments left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest undoing the changes in the following files and moving them to a separate PR against the protocol_changes branch:

  • EntityItemProperties.cpp/.h
  • EntityPropertyFlags.h (just delete the prop entirely)
  • ZoneEntityItem.cpp/.h (remove the ignoreUint32 thing)
  • PacketHeaders.h (no need to rename the old version, just add a new version at the end)

with the ignoreUint32 hack, this might not crash, but it's not worth the complexity/risk imo.

@HifiExperiments
Copy link
Member

hope you don't mind but I took the opportunity to rebase this once again and really remove the property and bump the protocol (on the protocol_changes branch): #1165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants