-
Notifications
You must be signed in to change notification settings - Fork 243
Marketplace prefab #1048
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
Marketplace prefab #1048
Conversation
browsing bugs fixed
juans-chainsafe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a Event System into a new scene and the MarketplaceLogin.prefab to test.
But I get different errors.
- After login:
NullReferenceException: Object reference not set to an instance of an object
Scripts.EVM.Remote.CSServer.GetDataWithToken[T] (System.String _path, System.String _bearerToken, System.Int32 offset, System.Int32 pageSize) (at /Users/juanmanuelspoleti/Desktop/workspace/web3.unity/Packages/io.chainsafe.web3-unity/Runtime/Scripts/EVM/Remote/CSServer.cs:49)
Scripts.EVM.Marketplace.Marketplace.GetProjectMarketplaces (System.String bearerToken) (at /Users/juanmanuelspoleti/Desktop/workspace/web3.unity/Packages/io.chainsafe.web3-unity.marketplace/Runtime/Scripts/Marketplace/Marketplace.cs:31)
ChainSafe.Gaming.Marketplace.BrowseMarketplaceManager.GetProjectMarketplaces () (at Assets/Samples/web3.unity SDK Marketplace/2.6.1/Marketplace Samples/Scripts/BrowseMarketplaceManager.cs:63)
System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) (at <e307bbb467104258887a104f6151f183>:0)
UnityEngine.UnitySynchronizationContext+WorkRequest.Invoke () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:153)
UnityEngine.UnitySynchronizationContext.Exec () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:83)
UnityEngine.UnitySynchronizationContext.ExecuteTasks () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:107)
- Trying to create a Marketplace
NullReferenceException: Object reference not set to an instance of an object
Scripts.EVM.Marketplace.Marketplace.CreateMarketplace (System.String _bearerToken, System.String _name, System.String _description, System.Boolean _whitelisting) (at /Users/juanmanuelspoleti/Desktop/workspace/web3.unity/Packages/io.chainsafe.web3-unity.marketplace/Runtime/Scripts/Marketplace/Marketplace.cs:334)
ChainSafe.Gaming.Marketplace.CreateMarketplaceManager.CreateMarketplace (System.String marketplaceName, System.String marketplaceDescription, System.Boolean marketplaceWhiteListing) (at Assets/Samples/web3.unity SDK Marketplace/2.6.1/Marketplace Samples/Scripts/CreateMarketplaceManager.cs:41)
System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) (at <e307bbb467104258887a104f6151f183>:0)
UnityEngine.UnitySynchronizationContext+WorkRequest.Invoke () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:153)
UnityEngine.UnitySynchronizationContext.Exec () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:83)
UnityEngine.UnitySynchronizationContext.ExecuteTasks () (at /Users/bokken/build/output/unity/unity/Runtime/Export/Scripting/UnitySynchronizationContext.cs:107)
-
Also, description for Marketplace should be optional, is not mandatory to add a description
-
Create a collection asks me for 4 images, but it should be only 2
|
Login token error is web3auth wallet related, needs a null check, will fix in another PR as it's out of scope. Marketplace description & name has been made optional. Image uploads should be at 2 images now instead of 4. |
processing bool was missing a reset after execution.
| public string cursor { get; set; } | ||
| public List<Item> items { get; set; } | ||
| public List<Owners> owners { get; set; } | ||
| #region ProjectMarketplaces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to Samples too (if you still need it), otherwise, we are going to have two implementations of the same thing in the package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, I'm talking about the whole implementation, not just this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will clean it up in next PR, too many changes in this one.
| namespace ChainSafe.Gaming.Marketplace | ||
| // <copyright file="MarketplaceItemStatus.cs" company="PlaceholderCompany"> | ||
| // Copyright (c) PlaceholderCompany. All rights reserved. | ||
| // </copyright> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this placeholder header pls and update from dev. After you update, lint.bat will no longer generate this bs in the files of Marketplace project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| MarketplaceItemStatus? filterStatus = null, | ||
| MarketplaceSortType sortType = MarketplaceSortType.None, | ||
| MarketplaceSortOrder? sortOrder = null) | ||
| public Task<MarketplacePage> LoadPage(MarketplacePage? prevPage = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it overwrites the recent marketplace update. Please make sure this doesn't get written to dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
This reverts commit d65d9ac.
resetting marketplace scripts
|
conflicts are messy, need to fix |
|
closing for #1057 |
MarketplacePrefab.mp4
To Test:
Drop the prefab from marketplace samples into a scene with an event manager, click around, have fun, woot woot.
I want to swap the calls to use the marketplace client calls but it will require some refactoring. I'd prefer to test basic functionality first and then optimize and cleanup in the next PR to keep it granular.