serge-web / serge

Serious Gaming, Evolved - web interface
https://sites.google.com/deepbluec.com/serge/
Apache License 2.0
14 stars 4 forks source link

Core mapping - Admin Page #2970

Open lebaphi opened 1 month ago

lebaphi commented 1 month ago

Fixes #2971

IanMayo commented 1 month ago

Hello @lebaphi - the PlayWright testing is currently failing for this. That's quite understandable, since so much as changed.

So, I suggest we (well, you ;-) ) change the CI workflow to comment out the PlayWright testing until this branch is mature, then we will reinstate it.

Tristina1788 commented 1 month ago

I can see 'Core Mapping' from admin page of game 'P10-Test' . But we still in implementing, so I don't check more on this.

image

IanMayo commented 1 month ago

Nice work so far @lebaphi 😀

Tristina1788 commented 1 month ago

@IanMayo Do you think we should change the 'Delete' icon to '...' icon. And we can have 2 actions (Delete / Duplicate) for each renderer . So that, we can create duplicated renderer and edit properties which we want to change. So it save time for creating.

image

IanMayo commented 1 month ago

@IanMayo Do you think we should change the 'Delete' icon to '...' icon. And we can have 2 actions (Delete / Duplicate) for each renderer . So that, we can create duplicated renderer and edit properties which we want to change. So it save time for creating.

Thanks @Tristina1788 - that's a good idea, but we currently only have the two renderers and don't expect them to be duplicated. Let's defer the suggestion for the future.

Tristina1788 commented 1 month ago

@IanMayo @lebaphi

image

image

image

image

image

image

https://github.com/serge-web/serge/assets/107697044/dd7344c0-e553-44b1-8635-cee836cc9828

Tristina1788 commented 1 month ago

@lebaphi @IanMayo

image

image

Tristina1788 commented 1 month ago

@lebaphi @IanMayo We should remove column 'Icon'. And we still don't have the way to update 'Others' because it's not in edit form

image

image

https://github.com/serge-web/serge/assets/107697044/e5421b40-06f5-47e2-ac08-61134d9f14b2

image

IanMayo commented 1 month ago

I've just done some testing @lebaphi - it's looking good. Note: for the EnumProperty the game designer needs to provide a list of values (choices), and for NumberProperty there should be an optional Format attribute that controls how the numbers are shown.

Oh, and there should be a lines attribute for a StringProperty.

For string and enum properties there should be a string default attribute, and a numeric one for number properties.

Tristina1788 commented 1 month ago

@lebaphi User role with 'View Props' = false, 'Edit/Remove Feature' = false, 'Move/Resize Feature' = false, user still can see props and edit/remove , move/resize feature

https://github.com/serge-web/serge/assets/107697044/a87874e6-2db5-4a00-95f7-e028da710e31

IanMayo commented 1 month ago

@lebaphi User role with 'View Props' = false, 'Edit/Remove Feature' = false, 'Move/Resize Feature' = false, user still can see props and edit/remove , move/resize feature

Thanks for the @Tristina1788 . I'll start off on the failure to use the props permission. The mapping permissions may require some trickery we haven't done before (allowing a user to move features from one force but not the other).

Tristina1788 commented 1 month ago

@lebaphi @IanMayo The list roles in force is not correct

image

image

IanMayo commented 1 month ago

@lebaphi @IanMayo The list roles in force is not correct

Yes, we should show the role name, rather than id

IanMayo commented 1 month ago

Hello @lebaphi - for the mapping channel editor in the admin pages, the text inside the badges/lozenges is really small. I know you had to do that to fit them in the UI. But, it is possible to trim down the whitespace padding in the badge, so the text can be a little larger? image

Tristina1788 commented 1 month ago

@lebaphi @IanMayo I get white screen after updating properties of renderer.

Steps :

  1. Update a property of a renderer
  2. Logout and login gameplay
  3. Click on renderer which has updated for renderer
  4. Notice the white screen

https://github.com/serge-web/serge/assets/107697044/4b4253c9-840a-4321-ba53-852d7dde756d

Stesp:

  1. Update Max Native Zoom = 1
  2. Update Max Zoom = 2
  3. Notice Max Native Zoom is reverted to original value (12)

https://github.com/serge-web/serge/assets/107697044/466a1301-d30e-46c2-b250-bac906801a11

Tristina1788 commented 1 month ago

@lebaphi @IanMayo

image

https://github.com/serge-web/serge/assets/107697044/6f566818-56b2-4a1c-acf2-b797461652fd

IanMayo commented 1 month ago

@Tristina1788 - the new renderer issue should be fixed in my last commit.

Tristina1788 commented 1 month ago

@Tristina1788 - the new renderer issue should be fixed in my last commit.

@IanMayo Yes, the issue on new renderer is fixed now : https://app.screencast.com/NwfECxUDvz7v6

IanMayo commented 1 month ago

So is this ready to go @Tristina1788 ?

Tristina1788 commented 1 month ago

@IanMayo @lebaphi We still have some issues here :

image

image

image

image

image

lebaphi commented 1 month ago

We should not limit zoom value from 1->20 , it should be the number > 0 and don't have limit for max

if so, i think we shouldn't use dropdown list, it should be input text. What do you think @IanMayo @Tristina1788 ?

Tristina1788 commented 1 month ago

@lebaphi @IanMayo

https://github.com/serge-web/serge/assets/107697044/cf82f01c-9605-4b47-96ea-e3323a183cdf

https://github.com/serge-web/serge/assets/107697044/db62f6be-3f33-421c-96a8-320c0a709130

https://github.com/serge-web/serge/assets/107697044/0c1f9e38-da68-4678-8a03-03692fdd813e

lebaphi commented 1 month ago

image

=> the channel data does not have the green force, so we don't display it here

image

=> some participants does not have role, so what do you think @IanMayo ?

image

=> We can add tooltip and @IanMayo can design details for each permission, but only admins can access this page and I think admin are the one who understand what these permissions mean, do we need more tooltip?

image

=> this is because you are adding 2 core renderers => it renders 2 times. Should we allow to add 2 renderers which has the same kind @IanMayo ?

Tristina1788 commented 4 weeks ago

image

=> the channel data does not have the green force, so we don't display it here

@lebaphi @IanMayo I think 'Force' should get from 'Force' tab . So it should has all force or enable to add all force. How do you think?

image

Tristina1788 commented 4 weeks ago

@lebaphi @IanMayo

image

image

image

lebaphi commented 4 weeks ago

image => the channel data does not have the green force, so we don't display it here

@lebaphi @IanMayo I think 'Force' should get from 'Force' tab . So it should has all force or enable to add all force. How do you think?

image

it should be the force in channel participant I think

Tristina1788 commented 3 weeks ago

@lebaphi @IanMayo

image

image

image

image

image

image

https://github.com/serge-web/serge/assets/107697044/8e463175-3722-42d9-8e29-91f4c25e265f

https://github.com/serge-web/serge/assets/107697044/97e78b61-58d0-4125-89d2-c1f6a0723fbb

Tristina1788 commented 3 weeks ago

@IanMayo @lebaphi

https://github.com/serge-web/serge/assets/107697044/b167d202-7dd3-4da7-8af8-70cc7eca41c2

image

image

image

https://github.com/serge-web/serge/assets/107697044/46fee2fd-e094-4702-b003-00e2ed6869a7

Tristina1788 commented 3 weeks ago

@IanMayo @lebaphi

https://github.com/serge-web/serge/assets/107697044/54297dd8-d7a9-4a46-8eb5-eab78b583f36

image

lebaphi commented 3 weeks ago

Force dropdown list for Force properties should gets from 'Force' tab instead of input as enum values. @IanMayo @lebaphi How do you think? Because we can input incorrect name of Force and it will get invalid force to select.

This dropdown list is used for admin users to add or remove options for enum properties (we don't have this feature in the design), so we should not add constraints to it I think. What do you think? @IanMayo

IanMayo commented 2 weeks ago

Force dropdown list for Force properties should gets from 'Force' tab instead of input as enum values. @IanMayo @lebaphi How do you think? Because we can input incorrect name of Force and it will get invalid force to select.

This dropdown list is used for admin users to add or remove options for enum properties (we don't have this feature in the design), so we should not add constraints to it I think. What do you think? @IanMayo

Yes - this is a free-form control. In this case it is being used for force, but for now I'm ok with forcing the game-designer to enter the correct force id values.

Maybe we'll add a dedicated force control type in the future, but not now.

Any other issues on this PR @Tristina1788 ?

Tristina1788 commented 2 weeks ago

Force dropdown list for Force properties should gets from 'Force' tab instead of input as enum values. @IanMayo @lebaphi How do you think? Because we can input incorrect name of Force and it will get invalid force to select.

This dropdown list is used for admin users to add or remove options for enum properties (we don't have this feature in the design), so we should not add constraints to it I think. What do you think? @IanMayo

Yes - this is a free-form control. In this case it is being used for force, but for now I'm ok with forcing the game-designer to enter the correct force id values.

Maybe we'll add a dedicated force control type in the future, but not now.

Any other issues on this PR @Tristina1788 ?

@IanMayo We still has issues on 2 comments :

IanMayo commented 2 weeks ago

@IanMayo We still has issues on 2 comments :

Thanks @Tristina1788 - please help @lebaphi in getting them sorted.

Tristina1788 commented 2 weeks ago

@IanMayo We still has issues on 2 comments :

Thanks @Tristina1788 - please help @lebaphi in getting them sorted.

@lebaphi , @IanMayo Here are the remaining issues :

https://github.com/serge-web/serge/assets/107697044/7ee502d2-420a-44ee-a275-b146f333c711

image

https://github.com/serge-web/serge/assets/107697044/eda5172c-302a-4ec0-8dc6-d1e9b477aa4d

https://github.com/serge-web/serge/assets/107697044/b2e9e7a0-80c5-4cb6-8bc8-9c5ddcb2da7a

IanMayo commented 2 weeks ago

Thanks @Tristina1788 . Would you mind re-writing the paragraph that starts with The renderer shows on gameplay? I'm afraid I don't understand the issue.

Tristina1788 commented 2 weeks ago

Thanks @Tristina1788 . Would you mind re-writing the paragraph that starts with The renderer shows on gameplay? I'm afraid I don't understand the issue.

@IanMayo I mean the properties of 'Important' , 'Phase', 'Force', 'Turn' are only enable to get config from admin until user update anything on these properties. With 'Turn' it can get description, label from config after updating, but 'Editable' is always true and don't get value from config. (Example my screencast, notice 'Important' , 'Phase', 'Force', 'Turn' show as these properties are editable until I try to update description of these properties, then back to gameplay, now these properties show correctly with config (Editable = false)) . If you 're still confuse on this, I will give more details.

lebaphi commented 1 week ago

Created new participant and login game play with this user and draw a shape. It will make the exist shape / core are disappeared.

Hi @IanMayo, I've just investigate and found the root cause is that the way we handle messages:

What do you think?

IanMayo commented 1 week ago

Aah, thanks for debugging that @lebaphi

@Tristina1788 - could you edit the game (in admin pages) so that the umpire force can see green spatial data and props please?

Tristina1788 commented 1 week ago

Aah, thanks for debugging that @lebaphi

@Tristina1788 - could you edit the game (in admin pages) so that the umpire force can see green spatial data and props please?

@IanMayo @lebaphi Yes, if I added permission umpire force can see green, it will shows green items. But the issue is the exist item of red force, umpire force are lost.

lebaphi commented 1 week ago

@IanMayo @lebaphi Yes, if I added permission umpire force can see green, it will shows green items. But the issue is the exist item of red force, umpire force are lost.

Yep, that is the issue that I said in the 2nd part, because we only get last message in the list to render @IanMayo, you can see it here https://github.com/serge-web/serge/blob/develop/client/src/Components/local/atoms/core-mapping/index.tsx#L141

But the last message does not includes item of red or umpire because this message was created in green force. image

Tristina1788 commented 1 week ago

@IanMayo @lebaphi Yes, if I added permission umpire force can see green, it will shows green items. But the issue is the exist item of red force, umpire force are lost.

Yep, that is the issue that I said in the 2nd part, because we only get last message in the list to render @IanMayo, you can see it here https://github.com/serge-web/serge/blob/develop/client/src/Components/local/atoms/core-mapping/index.tsx#L141

But the last message does not includes item of red or umpire because this message was created in green force. image

@lebaphi , @IanMayo I think the exist items should be kept. (Example red force create some new items, login from Umpire force, the exist items and new items can be showed)

image

lebaphi commented 1 week ago

@IanMayo @lebaphi Yes, if I added permission umpire force can see green, it will shows green items. But the issue is the exist item of red force, umpire force are lost.

Yep, that is the issue that I said in the 2nd part, because we only get last message in the list to render @IanMayo, you can see it here https://github.com/serge-web/serge/blob/develop/client/src/Components/local/atoms/core-mapping/index.tsx#L141 But the last message does not includes item of red or umpire because this message was created in green force. image

@lebaphi , @IanMayo I think the exist items should be kept. (Example red force create some new items, login from Umpire force, the exist items and new items can be showed)

image

I think so, I just want to discuss with @IanMayo about the solution.

Tristina1788 commented 1 week ago

@lebaphi @IanMayo

image

https://github.com/user-attachments/assets/9e5756f3-90eb-44d3-a0b2-450f24907e22

lebaphi commented 1 week ago

Now the default properties on 'Core'/'Milsym' are correct with properties from UI. But after updating 'Editable' of MilSym's properties gets from Core, not from 'Milsym'

Hi @Tristina1788, to make the field editable, in addition to updating user editable field in Renderer tab, you need to update Edit permission in Participant tab. The field will be disabled if the user does not have Edit permission image

Tristina1788 commented 1 week ago

@lebaphi @IanMayo

https://github.com/user-attachments/assets/53002aed-9ac8-4d20-ae87-9f9d31dc89da

https://github.com/user-attachments/assets/2521e70b-034c-4980-9561-054cfd011ac7

https://github.com/user-attachments/assets/b3c29ee7-933a-4e57-bcd9-fb1bcfdc696b

image

image

image

https://github.com/user-attachments/assets/d6037f7f-39fb-4734-a17c-c95d41c9bb55

https://github.com/user-attachments/assets/7d9210cb-74a8-4cbb-bcb9-aa8228b0bb25

lebaphi commented 1 week ago

User doens't have permission 'View pros', 'Edit/remove Feature', 'Move/resize Feature' still can view pros, edit/remove item, move/resize item

I think this is an issue of @IanMayo 🤣

IanMayo commented 1 week ago

User doens't have permission 'View pros', 'Edit/remove Feature', 'Move/resize Feature' still can view pros, edit/remove item, move/resize item

I think this is an issue of @IanMayo 🤣

Sure @lebaphi - will look at it tomorrow morning

lebaphi commented 1 week ago

User doens't have permission 'View pros', 'Edit/remove Feature', 'Move/resize Feature' still can view pros, edit/remove item, move/resize item

I think this is an issue of @IanMayo 🤣

Sure @lebaphi - will look at it tomorrow morning

No worries @IanMayo, I fixed it

lebaphi commented 1 week ago

After remove properties, this property is still on the exist core/Milsym and show as ID. We should also remove the property on core/Milsym on gameplay.

When you create a new property and then create a new shape, it will be saved to the database (new shape with new property). Therefore, when you remove that property, "deleted" property still exists in the created shape, the new changes only applies to new shapes. If you want to fix this behavior, when an admin deletes a property, I think we should look up this property of each feature in the feature collection and then delete it. What do you think @IanMayo

IanMayo commented 1 week ago

After remove properties, this property is still on the exist core/Milsym and show as ID. We should also remove the property on core/Milsym on gameplay.

When you create a new property and then create a new shape, it will be saved to the database (new shape with new property). Therefore, when you remove that property, it still exists in the shape as a property. If you want to fix this behavior, when an admin deletes a property, I think we should look up this property of each feature in the feature collection and then delete it. What do you think @IanMayo

Aah, thanks for taking the time to explain that @lebaphi .

There isn't an easy answer. But, let's separate "game design" from "game play". The properties are most likely to change during the game design phase before features are added. Features will only really be created during game play. I do not believe we have to update/fix existing features to reflect properties being added or removed. The current set of "valid" properties are only observed when creating a new feature.