jdmonin / JSettlers2

Java Settlers project home, downloads, and GPLv3 source code. To download the latest version as a JAR, see https://github.com/jdmonin/JSettlers2/releases/latest .
http://nand.net/jsettlers/
GNU General Public License v3.0
160 stars 63 forks source link

setting scenario from command line overrides other options #88

Closed kotc closed 2 years ago

kotc commented 3 years ago

since there is no scenario randomizer yet, i'm trying to write a shell script that would do it. i'm having issue setting VP (and probably other options that selected scenario would set). what i've tried (wonders scenario is just an example):

1/ setting -o SC=SC_WOND -o VP=17 results in vp overridden 2/ setting -o _SC_WOND=t -o SBL=t -o _SC_SANY=t -o VP=17 results in no scenario set, not wonders map, although wonders feature is enabled).

am i doing something wrong?

jdmonin commented 3 years ago

Hi,

You've found an interesting behavior :)

When the scenario feature was being planned, the idea was that any options specified in the scenario's scOpts override any other options in the game, except VP if specified, because the scenario author knows best what options they need.

The bug you see in example "1" is that although the server tells the client there's a default VP=t17, the NewGameOptionsFrame is using VP=t10 specified in the SC_WOND scenario.

If you start the server with -o VP=t17 but no scenario, NewGameOptionsFrame does use 17 as default VP.

What example "2" runs into is: The options starting with '_' are internal, they shouldn't be set by default or requested by clients, so it's not surprising that behavior is inconsistent. They're meant to be set only when a game is created at the server (with a scenario or otherwise).

The bugfix for victory points would most likely need to happen in NewGameOptionsFrame, or possibly in SOCGameOptionSet.getAllKnownOptions at optSC.addChangeListener .

jdmonin commented 2 years ago

The upcoming v2.5 release's client will use default VP as a minimum when scenario has lower VP. Thanks for asking about this!

kotc commented 2 years ago

nice :) while you are at it, can you make that cmd line option override any automatic vp setting function? ie. if i set it to 17 no other function would touch it (apart from user manual vp setting in game create options)

jdmonin commented 2 years ago

Thanks :)

So, the intended behavior I've planned right now is:

Are you asking for a way to make sure each game's VP is always the default, even if the scenario specifies more, unless the user overrides it?

kotc commented 2 years ago

first two points yes, for third, if user doesnt change default vp in any way (cmd line or dropdown), then let scenario set the vp (ie. it doesnt matter if scenario vp < or > than default vp). when i play with gf we almost always use 17vp, its a setting that ensures you will have to use all the resources to win, there are draws sometimes, but that's ok too

kotc commented 2 years ago

idea: additional message could show (but not a popup) that will suggest what is the default vp for particular scenario, or just add it to selection list names

jdmonin commented 2 years ago

OK thanks, I think we're most of the way there with the upcoming v2.5 release.

If you're asking for a way to have the default VP always override scenario (Cloth Trade would be 13 instead of 14), I could add that. Although the highest scenario VP is 14, so if your command line is 14 VP or higher, it'll already do that in v2.5.

I like the idea of adding VP to the selection list names, thanks! Since v2.5 is so close to release, that would go into the next version after v2.5.

Does that sound good? Do you want a flag to always override the scenario, or would you be specifying >= 14 so it's alright without that flag? If I'm misunderstanding, just let me know :)

If no flag is needed, the current v2.5 client dialog code is all set, and I just need to finish a commit for the server side. I could also add the flag for v2.5.

kotc commented 2 years ago

since we are usually playing to 17 it would be ok, but if we ever start liking shorter games it would be nice if vp stuck to particular value. hmm, maybe a checkbox/cmdline option to disallow scenario changing the vp?

jdmonin commented 2 years ago

That checkbox/option sounds good to me too, yeah

jdmonin commented 2 years ago

Working on an implementation that'll add a boolean gameopt to use default VP for all scenarios. Should have something in a day or 3 :)

jdmonin commented 2 years ago

So if you have time and want to try this out:

Let me know if that fits what you're looking for. Thanks!

jdmonin commented 2 years ago

This was released as part of v2.5.00. We can iterate it a little afterwards if needed. :)

kotc commented 2 years ago

works well with server started separately, now it would be nice if client would offer the last selected VPs when creating new game after last one ended :)

jdmonin commented 2 years ago

Glad to hear :) Yeah, the New Game dialog still has some odd behavior with VPs sometimes. Definitely room for improvment there