search

smcameron / space-nerds-in-space

Multi-player spaceship bridge simulator game. Captain your starship through adventures with your friends. See https://smcameron.github.io/space-nerds-in-space
GNU General Public License v2.0
729 stars 73 forks source link

Engineering presets #255

Closed MCMic closed 4 years ago

MCMic commented 4 years ago

This adds 6 presets that the engineer can save and use as he pleases.

See #252

TODO:

Signed-off-by: Côme MCMic Chilliet come@chilliet.eu

MCMic commented 4 years ago

6 presets seems like a lot.

@smcameron I wanted to give the engineer a bit to play with. Standard usecase would be:

  1. All-off except life support
  2. All-on
  3. Cruising preset
  4. Warping preset
  5. Combat preset
  6. Something else more specific (science investigation, starbase approaching, mining preset, …)

(I actually hesitated between 6 and 8, and chose 6 to force the engineer to make some choices)

smcameron commented 4 years ago

Thanks! That was quick.

Ok, looking at your patches now... so far looks really good. Packing everything in the power_device_device with only uint8_t... you didn't even have to change the packet building code, other than the size, nice.

To get snis_multiverse to save and restore the data across sessions, I think you only need to update snis_entity_kvs[] in snis_entity_key_value_specification.h ... maybe you mean to do that in another patch.

The only other thing I noticed was that initially, no preset is active, but if I save, it saves to the 1st preset, so that is a minor inconsistency. Not sure how best to resolve it. If the 1st preset is active initially, the data it contains won't match the initial settings of the ship, so if you select the 1st preset without saving, you will get different results than what is currently shown (hope I am making sense) which is a bit counterintuitive. But maybe getting multiverse to save/restore the data across sessions will help that.

MCMic commented 4 years ago

To get snis_multiverse to save and restore the data across sessions, I think you only need to update snis_entity_kvs[] in snis_entity_key_value_specification.h ... maybe you mean to do that in another patch.

I did not try the restore across session I assumed it would work since it was it the same struct as the rest. I will look into it.

The only other thing I noticed was that initially, no preset is active, but if I save, it saves to the 1st preset, so that is a minor inconsistency. Not sure how best to resolve it. If the 1st preset is active initially, the data it contains won't match the initial settings of the ship, so if you select the 1st preset without saving, you will get different results than what is currently shown (hope I am making sense) which is a bit counterintuitive. But maybe getting multiverse to save/restore the data across sessions will help that.

Hum yeah I guess I need to activate preset 1 or disable save button.

smcameron commented 4 years ago

I'm going to go ahead and commit this as it stands because it is already an improvement.

I made one minor formatting tweak.

MCMic commented 4 years ago

@smcameron We should wait before default preset are added back no?

smcameron commented 4 years ago

We should wait before default preset are added back no?

lol, too late! Already committed.

smcameron commented 4 years ago

Committed as:

MCMic commented 4 years ago

@smcameron Why does init_engineering_ui declares a pointer instead of working on eng_ui directly like the other functions? (and it does use eng_ui.gauge_radius directly)

MCMic commented 4 years ago

pushed https://github.com/MCMic/space-nerds-in-space/commit/72f417f858b74987021739315fec998ad64aa9dc for the inconsistency at startup problem: it disables save until some preset is selected.

smcameron commented 4 years ago

Why does init_engineering_ui declares a pointer instead of working on eng_ui directly?

No real good reason, probably just "eu->" is shorter to type and takes up less screen space. No other reason.

smcameron commented 4 years ago

I noticed key_value_parser.[ch] doesn't know how to deal with arrays. I should probably teach it at least byte arrays.

smcameron commented 4 years ago

I noticed key_value_parser.[ch] doesn't know how to deal with arrays. I should probably teach it at least byte arrays.

Or maybe not (too lazy).

This commit allows engineering presets to be saved/restored by snis_multiverse and thus persisted across sessions.

smcameron commented 4 years ago

Because of the way key_value_parser.c works, if we change ENG_PRESET_NUMBER to something other than 6, we need to also change snis_entity_key_value_specification.h, so I put in a BUILD_ASSERT to catch it.

MCMic commented 4 years ago

Thanks. Can you push MCMic@72f417f as well?

Where should I look to add default presets? Seems like init_player is not it, and add_ship neither. add_player seems quite empty, but maybe that’s it? Is there no other cases of stuff that needs to be initialized at player ship creation, but not changed between sessions/respawns? Or should respawn lose this data because the ship is destroyed?

smcameron commented 4 years ago

Thanks. Can you push MCMic/space-nerds-in-space@72f417f as well?

Done:

smcameron commented 4 years ago

Where should I look to add default presets?

Probably add_new_player(), around where it says "/ didn't find our bridge, make a new one. /"

smcameron commented 4 years ago

Actually, that would probably be in add_player(), which is only ever called from add_new_player().

smcameron commented 4 years ago

I guess the presets should survive respawn.

MCMic commented 4 years ago

@smcameron Here for default presets: https://github.com/MCMic/space-nerds-in-space/commit/c56bd913553cb264c4246f738cc1b9d7e5f46756

It seems to me that maybe the lines setting all r1 to 0 here are an error: https://github.com/smcameron/space-nerds-in-space/blob/master/snis_server.c#L10594 Because it feels like it would make more sense to set r2 here, looking at the comment regarding lifesupport. But it’s overridden anyway by the calls to init_power_model just after, which sets all r* values again, and actually cuts of the lifesupport, so when creating a ship you are without oxygen. (but that saves fuel, I guess?)

smcameron commented 4 years ago

Yeah, all those settings for r1 in init_player seem like they can be nuked because snis init_power_model() resets them all.

The power and coolant sliders on engineering adjust all the r2 values. r2 forms a minimum value for r1, as power_model_update_resistances() enforces that r1 >= r2.

Note that this is after they have been through the power model sampler functions, defined by a hairy macro:

#define DECLARE_POWER_MODEL_SAMPLER(name, model, which) \
static float sample_##model##_##name##_##which(void *cookie) \
{ \
        struct snis_entity *o = cookie; \
        float v = 255.0 - (float) o->tsd.ship.model.name.which; \
\
        if (v > 250.0) \
                v = 10000.0; \
        v =  v * 10000.0; \
        return v; \
        /* return (float) 255.0 / (float) (o->tsd.ship.power_data.name.which + 1.0); */ \
        /* return (float) (256.0 - (float) o->tsd.ship.power_data.name.which) / 256.0; */  \
}

"name" is "warp", "maneuvering", etc. "model" is going to be "power_data" or "coolant_data" "which" is going to be "r1", "r2", or "r3"

it subtracts from 255 to reverse the sense of the 0 to 255 range, and adds 1 (which means you can never get zero resistance, which is why it doesn't divide by zero in power_model_compute().

the bit about "if (v > 250) v = 10000;" is just simulating infinite resistance if you turn the thing off.

I know in some of the old videos I used to have to turn everything on in the beginning. Now, previous power settings are saved and restored by multiverse, so if you crank up a ship you've been using before, the power comes on automatically back to what it was last time you ran the ship.

Incidentally, I think there's a memory leak in power-model.c, there are a couple allocations in new_power_model() and new_power_device(). It looks like I meant to free them at the beginning of init_power_model(), and init_coolant_model(), but it's commented out. git annotate says they've been commented out from the time those functions appeared. I don't know if there's a real reason for those to be commented out, looks wrong to me.

Some of the systems do not have any power control separate from engineering (tractor beam, lifesupport, comms, for these r1 is set to 255 which means "maximum power" (after going through the sampler function it 255 becomes minimum resistance) because there is no UI slider controlling r1 for these. We could get rid of the shields control slider for r1 in the same way, if we wanted to.

The main engineering power and coolant sliders control r2, with 255 meaning max power.

It's quite possible the initial values for a new ship don't make sense in some cases. I think they tend towards everything is turned off at engineering (I seem to remember this was one reason I put the origianal presets in, I got tired of having to turn everything on manually.)

With snis_multiverse saving and restoring the values across sessions, the initial values are less of a concern.

If you're looking for default values to use for presets, I suppose the old preset 1 values might work:

-       /* a "normal" preset, note only one poke has sound to avoid noise */
-       snis_slider_poke_input(eng_ui.shield_slider, 0.95, 1);
-       snis_slider_poke_input(eng_ui.shield_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.maneuvering_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.maneuvering_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.warp_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.warp_coolant_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.impulse_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.impulse_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.sensors_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.sensors_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.comm_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.comm_coolant_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.phaserbanks_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.phaserbanks_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.tractor_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.tractor_coolant_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.lifesupport_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.lifesupport_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.tractor_coolant_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.shield_control_slider, 1.0, 0);

1.0 equates to 255, 0 equates to 0, 0.95 equates to 242.

Preset 2 was

-       /* an "all stop" preset, note only one poke has sound to avoid noise */
-       snis_slider_poke_input(eng_ui.shield_slider, 0.0, 1);
-       snis_slider_poke_input(eng_ui.shield_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.maneuvering_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.maneuvering_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.warp_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.warp_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.impulse_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.impulse_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.sensors_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.sensors_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.comm_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.comm_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.phaserbanks_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.phaserbanks_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.tractor_slider, 0.0, 0);
-       snis_slider_poke_input(eng_ui.tractor_coolant_slider, 0.3, 0);
-       snis_slider_poke_input(eng_ui.lifesupport_slider, 0.95, 0);
-       snis_slider_poke_input(eng_ui.lifesupport_coolant_slider, 1.0, 0);
-       snis_slider_poke_input(eng_ui.shield_control_slider, 0.0, 0);

0.3 equates to 76

Hope that all makes sense.