OpenWF/SpaceNinjaServer
12
6
Fork 14
Code Issues 46 Pull Requests 1 Actions Packages Projects Releases Activity

chore: simplify conversion of missionReward from PE+ #1018

Merged
Sainan merged 2 commits from reward-cleanup into main 2025-02-25 16:58:07 -08:00
Conversation 17 Commits 2 Files Changed 1 +2 -4
Sainan commented 2025-02-25 14:20:19 -08:00
Owner
Copy Link
No description provided.
Sainan added 2 commits 2025-02-25 14:20:19 -08:00
chore: missionReward.countedItems are never StoreItems, simplify this
All checks were successful
Build / build (18) (push) Successful in 37s
Details
Build / build (22) (push) Successful in 33s
Details
Build / build (20) (pull_request) Successful in 1m9s
Details
Build / build (20) (push) Successful in 1m8s
Details
Build / build (18) (pull_request) Successful in 1m4s
Details
Build / build (22) (pull_request) Successful in 33s
Details
33545acc0d
OrdisPrime commented 2025-02-25 16:33:27 -08:00
Owner
Copy Link

I would like to keep the current way as a sanity check.
Or at least warn about the case and throw or something.
If for some reason, the data should contain non store items, then the client will bug out receiving them as missionrewards.
So, the additional check doesn't harm and makes sure the items conform with the client's expected format.

I would like to keep the current way as a sanity check. Or at least warn about the case and throw or something. If for some reason, the data should contain non store items, then the client will bug out receiving them as missionrewards. So, the additional check doesn't harm and makes sure the items conform with the client's expected format.
Sainan commented 2025-02-25 16:35:37 -08:00
Author
Owner
Copy Link

The additional check does harm because /StoreItems/ may be a substring of a non-StoreItem. Also if we assume the data is inaccurate, then we have a lot of other problems.

The additional check does harm because `/StoreItems/` may be a substring of a non-StoreItem. Also if we assume the data is inaccurate, then we have a lot of other problems.
OrdisPrime commented 2025-02-25 16:50:35 -08:00
Owner
Copy Link

The additional check does harm because /StoreItems/ may be a substring of a non-StoreItem. Also if we assume the data is inaccurate, then we have a lot of other problems.

Why would /StoreItems/ which is specifically the identifier of a storeitem be part of a non-store item.

AT LEAST we should throw when a non-store item is found

Sure, drop the additional logic, but make sure the case never happens, data will never be perfect.

> The additional check does harm because `/StoreItems/` may be a substring of a non-StoreItem. Also if we assume the data is inaccurate, then we have a lot of other problems. Why would /StoreItems/ which is specifically the identifier of a storeitem be part of a non-store item. AT LEAST we should throw when a non-store item is found Sure, drop the additional logic, but make sure the case never happens, data will never be perfect.
Sainan commented 2025-02-25 16:52:01 -08:00
Author
Owner
Copy Link

Example: /Lotus/Types/StoreItems/AvatarImages/ImageGaussVED :)

I'd say the data is as close to perfect as it gets, it's just a matter of respecting the 'types'.

Example: `/Lotus/Types/StoreItems/AvatarImages/ImageGaussVED` :) I'd say the data is as close to perfect as it gets, it's just a matter of respecting the 'types'.
OrdisPrime commented 2025-02-25 16:53:33 -08:00
Owner
Copy Link

Example: /Lotus/Types/StoreItems/AvatarImages/ImageGaussVED :)

I'd say the data is as close to perfect as it gets, it's just a matter of respecting the 'types'.

But that's a store item.
I'm confused.

And in that case just the store item version would be returned, because it contains store item.
What am I missing, am I too tired?

> Example: `/Lotus/Types/StoreItems/AvatarImages/ImageGaussVED` :) > > I'd say the data is as close to perfect as it gets, it's just a matter of respecting the 'types'. But that's a store item. I'm confused. And in that case just the store item version would be returned, because it contains store item. What am I missing, am I too tired?
Sainan commented 2025-02-25 16:54:16 -08:00
Author
Owner
Copy Link

That's not a store item. The store item for it is /Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED.

That's not a store item. The store item for it is `/Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED`.
OrdisPrime commented 2025-02-25 16:54:40 -08:00
Owner
Copy Link

well, it contains /storeitems/ so its a store item? :D
Why is it not a store item?

well, it contains /storeitems/ so its a store item? :D Why is it not a store item?
Sainan commented 2025-02-25 16:55:04 -08:00
Author
Owner
Copy Link

That's exactly what I'm saying is wrong with the logic.

That's exactly what I'm saying is wrong with the logic.
OrdisPrime commented 2025-02-25 16:55:55 -08:00
Owner
Copy Link

Seems like this item does not have a non-store item counterpart. For some reason

Seems like this item does not have a non-store item counterpart. For some reason
Sainan commented 2025-02-25 16:56:34 -08:00
Author
Owner
Copy Link

Bro.

  • Normal: /Lotus/Types/StoreItems/AvatarImages/ImageGaussVED
  • Store item: /Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED
Bro. - Normal: `/Lotus/Types/StoreItems/AvatarImages/ImageGaussVED` - Store item: `/Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED`
OrdisPrime commented 2025-02-25 16:57:16 -08:00
Owner
Copy Link

Bro.

  • Normal: /Lotus/Types/StoreItems/AvatarImages/ImageGaussVED
  • Store item: /Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED

Ah so im too tired
I think its a mistake in their data. oof

> Bro. > - Normal: `/Lotus/Types/StoreItems/AvatarImages/ImageGaussVED` > - Store item: `/Lotus/StoreItems/Types/StoreItems/AvatarImages/ImageGaussVED` Ah so im too tired I think its a mistake in their data. oof
Sainan commented 2025-02-25 16:57:51 -08:00
Author
Owner
Copy Link

Yes, DE is wrong, it's surely not our code that's making stupid assumption.

Yes, DE is wrong, it's surely not our code that's making stupid assumption.
OrdisPrime commented 2025-02-25 16:57:54 -08:00
Owner
Copy Link

well, okey. if the data are bad, we are doomed to find more of these annoying bugs anyway.
KK. remove the logic.

well, okey. if the data are bad, we are doomed to find more of these annoying bugs anyway. KK. remove the logic.
Sainan merged commit 3945359e7d into main 2025-02-25 16:58:07 -08:00
Sainan deleted branch reward-cleanup 2025-02-25 16:58:07 -08:00
OrdisPrime commented 2025-02-25 16:59:27 -08:00
Owner
Copy Link

Yes, DE is wrong, it's surely not our code that's making stupid assumption.

you can't tell me you don't think that's a typing mistake.
the client's data have shown REPEATEDLY in the past how many syntax errors there are in the package, double properties, spelling mistakes in [Lotus/...] like ,Lotus/

Also, there are barely any cases like this, is it the only one?
I am 100% sure someone made an error there.

> Yes, DE is wrong, it's surely not our code that's making stupid assumption. you can't tell me you don't think that's a typing mistake. the client's data have shown REPEATEDLY in the past how many syntax errors there are in the package, double properties, spelling mistakes in [Lotus/...] like ,Lotus/ Also, there are barely any cases like this, is it the only one? I am 100% sure someone made an error there.
Sainan commented 2025-02-25 17:00:11 -08:00
Author
Owner
Copy Link

It's not a mistake on DE's part, and it's not the only instance of this.

It's not a mistake on DE's part, and it's not the only instance of this.
OrdisPrime commented 2025-02-25 17:00:49 -08:00
Owner
Copy Link

If anything, this example shows, that we cannot just remove /storeItems/ and think its the normal non-store variant.
It shows we ALWAYS have to look up typeName.

If anything, this example shows, that we cannot just remove /storeItems/ and think its the normal non-store variant. It shows we ALWAYS have to look up typeName.
Sainan commented 2025-02-25 17:02:36 -08:00
Author
Owner
Copy Link

It's fine, string.replace in JavaScript only replaces the first occurence so this generally works for us, and we have no correctness problems.

It's fine, string.replace in JavaScript only replaces the first occurence so this generally works for us, and we have no correctness problems.
👍 1
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
An existing implemention is incorrect
dependencies
Pull requests that update a dependency file
documentation
Improvements or additions to documentation
enhancement
New feature or request specific to the emulator
help wanted
Extra attention is needed
improvement
missing data
part of more major rehaul
Can't be worked on right now without merge conflicts later
pr'd for
Soon™
question
Further information is requested
unimplemented
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: OpenWF/SpaceNinjaServer#1018
No description provided.
Delete Branch "reward-cleanup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?