#2779 closed defect (fixed)
[PATCH] JS warnings in the game setup screen for guest players
| Reported by: | Stan | Owned by: | Itms |
|---|---|---|---|
| Priority: | Should Have | Milestone: | Alpha 17 |
| Component: | UI & Simulation | Keywords: | |
| Cc: | Patch: |
Description (last modified by )
When playing on the lobby I get those warnings. (See attached screenshot)
Attachments (2)
Change History (7)
by , 10 years ago
| Attachment: | Sans titre.png added |
|---|
comment:1 by , 10 years ago
| Component: | Core engine → UI & Simulation |
|---|---|
| Description: | modified (diff) |
| Keywords: | simple added |
| Milestone: | Alpha 17 → Backlog |
| Summary: | JS Warnings in the lobby → JS warnings in the game setup screen for guest players |
by , 10 years ago
comment:2 by , 10 years ago
| Keywords: | review patch added |
|---|---|
| Milestone: | Backlog → Alpha 17 |
| Summary: | JS warnings in the game setup screen for guest players → [PATCH] JS warnings in the game setup screen for guest players |
comment:4 by , 10 years ago
| Keywords: | simple review patch removed |
|---|
Thanks for that patch!
We're currently in feature freeze for the next alpha release, so sadly we didn't find time to review this patch quickly. The patch wasn't perfect but to save time I decided to correct it and commit, instead of making you improve it and learn things, so I apologize here. Fixing that is really needed for A17, which gets closer and closer!
I'll make comments here:
- The functionnality was not perfect, i.e. a problem was fixed indeed, but not the other one. The place you added a fix was not the faulty place for the "map selection" error. I encourage you to take a look at the committed code to see what actually failed.
- The code style was ok, don't hesitate to improve the current quality when you see some code doesn't follow the wiki:Coding_Conventions (we often make mistakes!) (I'm talking about the
if(()&&())instead ofif(() && ()), see your patch). We're used to writingif (foo)instead of the explicitif (foo !== undefined)when we can, for code readability, even if the latter is good.
We're looking forward more contributions from you, thanks for working on this! :)
comment:5 by , 10 years ago
Just want to mention, that if (foo) and if (foo !== undefined) behave different in case of foo having one of these values "", 0, NaN.
Note:
See TracTickets
for help on using tickets.
These are just warnings for data that are not well initialized when the player can't choose the map, the advanced options, etc.
This is not urgent and I think it's a good ticket for people willing to tackle their first bug. To fix that, the idea would be to add checks in the gamesetup.js code.