vircadia-native-core icon indicating copy to clipboard operation
vircadia-native-core copied to clipboard

Published 20 hours ago • verified publisher icon vircadia

Drop all dependencies on marketplace and wallet

Open odysseus654 opened this issue 4 years ago • 23 comments

This PR attempts to drop all references to the High Fidelity marketplace and wallet. Some effort has been made to prevent this from being a protocol-break (although there's a fair number of enums and entity packet content that could be pruned out once this is landed).

Going over the commits, things this thing removes:

  • All avatar and domain ownership challenges
  • Queries to FST files to see if they came from the marketplace
  • Permissions "can rez certified" and "can temprez certified"
  • The "Wallet" and "Marketplace" apps, along with the majority of the "Security" app
  • A context overlay of some kind showing the current marketplace status of any item
  • The "upload Avatar to Marketplace" tool
  • The scripting objects ContextOverlay and WalletScriptingInterface
  • 12 entity properties exclusively used for the marketplace (still kept in the protocol stream for compatibility, but unused)
  • Assignment Clients being paid for their work (related: the -wallet option on assignment-client)

Note that I did pick up on an apparent typo in AssignmentClientApp that on its surface appears to effectively disable the -a and -p options; I'm sorely tempted to break this up into its own patch for independent review (it must stay as part of this patch as well though)

odysseus654 avatar Aug 30 '20 23:08 odysseus654

ContextOverlay may or may not have been useful for security reasons? We should look into that to ensure there's not some usefulness to having something like that regardless.

two-one-five avatar Aug 30 '20 23:08 two-one-five

Yah that was a bit confusing to me too. Everything I saw had it keyed to ownership challenges for objects though, which was square in the crosshairs of this PR. Once I dropped those there was basically nothing left. I don't really know the value of empty containers at this point. Heck I think I may have left an empty .cpp file in this project after stripping out all the code...

odysseus654 avatar Aug 30 '20 23:08 odysseus654

MixerAvatar.cpp likely can be deleted as well. MixerAvatar.h is still in use and appears to have value (even though 90% of it has been removed)

odysseus654 avatar Aug 30 '20 23:08 odysseus654

The following links are available: build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-0ddb89a_c26bec3a.exe

build (macOS-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-0ddb89a_c26bec3a.dmg

build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-0ddb89a.tar_c26bec3a.Z
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-0ddb89a.tar_c26bec3a.gz
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-0ddb89a_c26bec3a.sh

vircadia-build-notifier avatar Aug 31 '20 02:08 vircadia-build-notifier

Have rebased & squished the commits in this PR. Will compile/smoketest as well as remove MixerAvatar.cpp

odysseus654 avatar Sep 06 '20 23:09 odysseus654

Smoketested the interface and dropped the empty MixerAvatar.cpp file. Note that I have not yet done any testing of the server.

odysseus654 avatar Sep 07 '20 00:09 odysseus654

The following links are available: build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-f91e841.tar_b9dbda9c.gz
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-f91e841.tar_b9dbda9c.Z
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-f91e841_b9dbda9c.sh

build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-f91e841_b9dbda9c.exe

build (macOS-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-f91e841_b9dbda9c.dmg

vircadia-build-notifier avatar Sep 07 '20 02:09 vircadia-build-notifier

Could you get rid of the merge conflicts so I can build and test this? I cannot view the conflict, so I have no idea how big of a change it might be.

JulianGro avatar Sep 23 '20 16:09 JulianGro

Rebased / squished the commits in this PR. Have compiled / smoketested to check for any obvious issues.

odysseus654 avatar Oct 04 '20 03:10 odysseus654

The following links are available: build (macOS-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-5535bf8_3a4051bd.dmg

build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-5535bf8.tar_3a4051bd.gz
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-5535bf8_3a4051bd.sh
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-5535bf8.tar_3a4051bd.Z

build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/kasenvr_project-athena/PR666/Vircadia-Alpha-PR666-5535bf8_3a4051bd.exe

vircadia-build-notifier avatar Oct 04 '20 05:10 vircadia-build-notifier

Building works fine at the very least https://appimage.moto9000.moe/experimental/PR666/Vircadia-x86_64_PR666_3a4051b.AppImage

JulianGro avatar Oct 04 '20 12:10 JulianGro

I haven't had any problems with this yet.

JulianGro avatar Oct 08 '20 22:10 JulianGro

I think it would be better, as a first step, to disable items (e.g., don't show UI and/or comment out code) rather than a wholesale deletion of code - some of which may prove useful in the future (either as actual code or implementation hints) should we want to address similar functionality. Some code might be able to be removed after considering it, piece by piece.

ctrlaltdavid avatar Oct 09 '20 21:10 ctrlaltdavid

This deletion approach was done as it tends to be a bright-line describing all the code in the system that depends on the wallet in some form. Commenting out code can be a fine line between keeping code that may be useful in the system and keeping significant junk in the codebase that will never be used in the future. I do recognize the amount of loss that is involved in this patch though; a lot of the deletions I've made represented unrealized potential that I would really have liked to have seen happen.

If some of this code has value to be kept in the system then yah let's keep it in (potentially commented out). Such decisions would have to be intentional though; the code is linked together enough that cauterizing parts of it out would take a bit of effort.

I would not be against somehow breaking this PR up into smaller sub-PRs (and smaller decisions on what to drop).

odysseus654 avatar Oct 13 '20 02:10 odysseus654

A series of smaller PRs, each focused on a particular area, may well be better. Even if, combined, they all end up removing the same code, at least we'll have merge commits in effect documenting code points likely to be involved if we want to re-enable or implement some particular feature at some later date.

ctrlaltdavid avatar Oct 15 '20 18:10 ctrlaltdavid

No major objections, PR #667 was originally part of this patch too. Would like to keep this PR open until a final decision is made on the whole (or the remaining) though.

odysseus654 avatar Oct 15 '20 19:10 odysseus654

Although breaking this up into smaller pieces would likely cause some extra work to be needed depending on the amount of effort required to cauterize the linkages. I wouldn't mind suggestions.

odysseus654 avatar Oct 15 '20 19:10 odysseus654

Dev meeting: it could be interesting to review why the merge is failing here -- this is code to remove, so why are changes being made that affect it?

daleglass avatar Jan 16 '21 19:01 daleglass

The following links are available: build (macOS-latest, client)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-Interface-PR666-f0e4ca0-_3dbaaf9e.dmg

build (macOS-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0-_3dbaaf9e.dmg

build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0-.tar_3dbaaf9e.Z
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0-_3dbaaf9e.sh
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0-.tar_3dbaaf9e.gz

build (ubuntu-18.04, android)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-Quest-Alpha-PR666-f0e4ca0_3dbaaf9e.apk
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0_3dbaaf9e.apk

build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-f0e4ca0-_3dbaaf9e.exe

vircadia-build-notifier avatar Apr 25 '21 05:04 vircadia-build-notifier

The following links are available: build (macOS-latest, client)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-Interface-PR666-bd481d1-_97d2dc56.dmg

build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-bd481d1-.tar_97d2dc56.Z
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-bd481d1-.tar_97d2dc56.gz
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-bd481d1-_97d2dc56.sh

build (macOS-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-bd481d1-_97d2dc56.dmg

build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-bd481d1-_97d2dc56.exe

vircadia-build-notifier avatar Aug 25 '21 05:08 vircadia-build-notifier

Whether it's broken up for removal or not, I think it's good to remove since it's not being used. If you think it might be useful to implement something in the future, you could then open a new PR adding those parts back in. That might be more subject to merge issues but I think there is practical value in streamlining the code base so it is more intuitive to new contributors. Hopefully this would also speed up the build process a bit.

I think there has been some talk of implementing hypothetical market/commerce functionality purely as a tablet application, or perhaps using a server-side plug-in or module, rather than building it into the application code. Correct me if I'm wrong.

Penguin-Guru avatar Dec 02 '21 17:12 Penguin-Guru

The following links are available: build (ubuntu-18.04, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-e0b209c-.tar_822a0982.gz
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-e0b209c-_822a0982.sh
  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-e0b209c-.tar_822a0982.Z

build (windows-latest, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-e0b209c-_822a0982.exe

build (macOS-10.15, full)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-PR666-e0b209c-_822a0982.dmg

build (macOS-10.15, client)

  • https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR666/Vircadia-Interface-PR666-e0b209c-_822a0982.dmg

vircadia-build-notifier avatar Jan 26 '22 15:01 vircadia-build-notifier

Hello! Is this still an issue?

stale[bot] avatar Jul 26 '22 00:07 stale[bot]