New game start dialog #5561
New game start dialog #5561
Conversation
|
What's the license of https://github.com/thomasleveil/name-generator ? I don't find any License file. |
|
Restoring obsolete saves? This is fabulous news - thank you, looking forward to it. If you are keeping a list of objectives, may I please suggest that the feature is able to import previous flight logs? That is one element that cannot be 'cheated' via the Lua Console. |
|
I can already tell this will be a fun review process - from a quick skim (and previous looks at e.g. sector map separation) I'm liking the code I see so far. There may be a few patterns employed in the substantial amounts of new code in this PR which I may ask for a refactor/improvement on, but I won't have any concrete examples until I start going through the detailed review process. I'll start by mentioning the biggest and most onerous task here: the list of names used for ships needs to be pruned to remove any 'handles' (TwoWord style names, e.g. BarryNelson) and any immediately recognizable character names from works of fiction outside the Pioneer universe or more generally covered under intellectual property laws (e.g. C3PO, Daenerys, Galadriel, etc.). This is something that will have to be done manually, as there are a lot of offending entries in that file. I know we're not a commercial entity and the chance of getting into any actual trouble related to IP law is slim-to-none, but these names set the tone for Pioneer's universe as one of the first "in-universe" elements the player sees (on the start screen, even!) and thus a significant amount of care needs to be taken here. 'Handle' style names can be split into their component "real world" names and added as two separate entries at your discretion, (e.g. Finally, ship names are internationally considered Proper Nouns, and thus the left and right components of a ship name should both have the initial letters capitalized. (...Hopefully I've bought myself enough time here to make headway on the detailed review 😅) |
|
Made some fixes - just comments, and some unused code |
Gliese852
force-pushed
the
new-game-menu
branch
from
9200dad
to
7dabfc9
Compare
There was a problem hiding this comment.
I've addressed some low-hanging fruit in several areas, but will defer review of the SectorMap and main game start dialog to the next batch of review comments. I need to spend some time reading that code and see if there are some prior comments/concerns discussed on IRC that still need addressing.
Well done on what I've reviewed so far!
|
@craigo- thanks for support! I think this can be implemented (import previous flight logs) |
|
@Web-eWorks about the list of words - I think for a while to wait a bit, because it is not clear about the license. I asked the author a question, but he is still silent. Maybe I'll have to clean up the whole list and look for another. |
Alternative, there was a request on SSC, to export flight log to txt file. It's a feature I'd like to see. |
|
A few issues I'm noting here:
Similarly on the ship page:
|
There was a problem hiding this comment.
I've left a few (11) change requests for the Lua code aspect of things. I've taken a fairly high-level look at the Lua code and tried not to nitpick the architectural design too closely; at this point I think you write high-quality code and I don't need to "police" it.
The design of GameParam is acceptable, though the "locking mode" may need a second thought as I'm not seeing any uses for lock mode other than on or off (and future special "locked" modes should probably be initialized from the start preset and the locked boolean used only to determine whether the lock mode should apply or not).
Regarding the C++ side of things, I want to say there may be a slightly more performant way to handle SectorMapContext::Callbacks::GetDisplayMode() than a virtual function dispatch, but at the current juncture I have no issues with it. Otherwise the code looks fairly good and I'm quite happy with the cleanup you've done in various files!
| end | ||
| end | ||
|
|
||
| local function startGame(gameParams) |
There was a problem hiding this comment.
Ideally this function should provide a way for modules which provide additional start variants etc. to perform initial setup on the player's ship / universe state, perhaps by calling an event handler and passing the gameParams value.
This was done, one might say, intentionally, on the one hand, this is a parameter, it’s just that its editing is blocked, on the other hand, I was too lazy to insert plain text there and add a background to it, calculate all this, etc. If you think it's important, I'll change it. |
|
@Web-eWorks I noticed that the game may crash when starting in a multiple system, apparently this was already fixed in the c58d3bb, this branch just budged earlier. |
|
Tried to filter (to the extent of knowledge) the list of ship names. |
There was a need to use the sector map in different contexts, not only
inside the SectorView. Therefore, the 3d star map viewer has been moved
to a separate SectorMap class. The SectorMap class has an interface for
adding custom objects to the map - lines, spheres and round indicators.
The SectorView uses this to overlay game logic objects (player location,
route, etc.) on the map. Some functions that SectorView used to perform
directly are passed to SectorMap, and are performed via
GetMap()->Function(). The SectorMapContext class has also been added,
which contains all the necessary external information for the SectorMap
to function.
Also added the ability to display SectorMap inside the ImGui window.
Some additional modifications:
* completely get rid of Pi:: in the SectorMap class
- move GetMoveSpeedShiftModifier() to Input::Manager
- explicitly pass frame time to SectorMap::Update
* input tweaking
- receive wheel data directly instead of via a signal
- draw arrow icons instead of '<' and '>' chars
- scale the face generator buttons proportionally to the fonts so that
it is more predictable in the game start menu
- make a public renderFaceGenButtons method so that it can be
controlled from outside
- add the ability to draw with or without a progress bar
- now it works well with complex formats like YYYY-MM-DD, will need it
- add mouse wheel control
- also apply small fix in ui.withButtonColors, to wrap the drag call
in this function
- pass any flags to the ColorEdit widget - simplify color passing to ModelSkin - fix the use cases
When setting the ModelSpinner projection, the FOV parameter means the vertical viewing angle by default. This causes the zoom to be dependent on the viewing height. Therefore, for example, in the ships market, where the height of the widget with the ship is small, the ships are drawn very small and there is a lot of free space on the sides. Changed to make FOV stand for the horizontal field of view. Now, it seems, the ships will be displayed more optimally.
- fix that ui.withStyleColors couldn't return multiple values - return value from ui.withButtonColors - round with double in format_money function
Also implement some functions for the convenience of obtaining a rating by its numerical value.
Visually, the dialog consists of widgets grouped into 4 tabs. The internal state is represented by a set of parameters sufficient to run the game. Widgets do not exactly match parameters - there may be widgets not represented by parameters, and parameters not shown as widgets, but most parameters correspond to some widget. Each parameter is an "instance" of some class, it can pull its data from multiple sources (built-in start variant, savegame document), display an interface for viewing/editing, validate it, and export the data to the game's start routine. Widgets have the property of blocking, by default editing most of the parameters is not available. In order to unlock all parameters, there is a 'Custom start' option, but it is disabled by default. When clicking on the "New Game" button, if Ctrl is pressed, an additional option "Custom start" will appear, unlocking all parameters. In this commit, the dialog is only added, it is fully functional, but the procedure for starting the game has not yet been implemented.
Add one argument to the game constructor so that it immediately creates a ship of the correct type, otherwise there were some problems with geoms. If not docked, the ship is spawned in a low circular orbit. Remove the ability to edit the face and name of the player inside the game. Slightly reduced the image in personal information. Do not create a player character at game start if one has already been created.
Gliese852
force-pushed
the
new-game-menu
branch
from
faea6b3
to
ccf50e5
Compare
Removed most "out of place" or trademarked names which readily associated with a fictional entity/character rather than a real name. The focus is mostly on preserving the tone and context of the world of Pioneer rather than using 100% original names. The criteria for remaining names is that they should not immediately stand out as belonging to a specific commercial work within the last ~50 years when combined with another name fragment. Capitalized all name fragment lists, as the resulting generated word will be an english Proper Noun and should be capitalized.
|
Well done @Gliese852! Thank you for your patience and the large amount of work that went into this PR! |
|
Saves after commit 032e80b are broken with following backtrace: |
|
@Gliese852 Brilliant new start screen! I got this glitch. Sometimes the fields 'Name' and 'Cash' overlap. I haven't seen a pattern in this but it's happened twice. |
Visually, the dialog consists of widgets grouped into 4 tabs. The internal state is represented by a set of parameters sufficient to run the game. Widgets do not exactly match parameters - there may be widgets not represented by parameters, and parameters not shown as widgets, but most parameters correspond to some widget.
Each parameter is an "instance" of some class, it can pull its data from multiple sources (built-in start variant, savegame document), display an interface for viewing/editing, validate it, and export the data to the game's start routine.
Widgets have the property of blocking, by default editing most of the parameters is not available. In order to unlock all parameters, there is a 'Custom start' option, but it is disabled by default. To enable it, you need to activate the 'Debug start dialog' checkbox in the debug window.
First of all, the purpose of this menu is the basis for restoring obsolete saves. Although in this PR, such an feature is not yet available. As the next step, it is planned to add the ability to import the state from the savegame. And if some parameters cannot be imported, allow them to be edited manually.
In addition, skins for starting ships were fixed:
Specific skins are, of course, a matter of debate.
Also added a simple random name generator for the ship, lists are taken from here: https://github.com/thomasleveil/name-generator/tree/master/data