Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scenarios #8501

Merged
merged 8 commits into from Aug 25, 2014

Conversation

Projects
None yet
7 participants
@BeigeSand
Copy link
Contributor

commented Aug 9, 2014

Not sure why there's merge conflicts, it probably has something to do with the files I added. If there's something I should do to fix it let me know. Probably involves adding the file names to some list or something if i had to guess.

Update:Scenarios assign starting location, and limit your available professions on the scenario chosen. The available professions can be easily changed by adding them to the professions array in scenarios.json, or all of them can be made accessible by adding "ALL_PROFS" to the flags for a scenario. All scenarios currently in the .json file are for testing purposes only.

TODO:
Clean up comments (Some left over from profession).
Assign traits to scenarios.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Aug 10, 2014

Neat, I kind of feel like it's more appropriate to discuss on the forum thread http://smf.cataclysmdda.com/index.php?topic=7434.0 since they're in such a nascent state. I don't think that invalidates anything you have here, which at a glance looks good.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2014

My current implementation of the scenario/profession interlocks is on this branch at the moment. Devs feel free to toy with it and see if it's too your liking, I'm all too happy to make changes if the current implementation isn't up to snuff. I'm pretty sure I've fixed all the major bugs in UI, but it would not surprise me if one creeps up.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2014

Alright, all that should be left for this PR is cleanup, I'll throw up comment fixes and some standard scenarios tommorow, but that should be about it. Let me know if i'm forgetting anything.

@BeigeSand BeigeSand changed the title Scenarios[WIP] Scenarios[CR] Aug 15, 2014

"ident": "bitten",
"name": "Ow...",
"points" : -4,
"description": "You dun messed up A-Aron, you went and got yo' ass bitten.",

This comment has been minimized.

Copy link
@HuXTUS

HuXTUS Aug 15, 2014

Contributor

ass bitten. Cool. )

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2014

Yeah, these were just for testing purposes, some real scenarios up now. The lab start seems to be busted though, can't ever find one. I suspect surface labs are assigned some other ident than "lab", but can't find where it would be doing it, and haven't guessed a correct one yet.

"rucksack",
"scar_h",
"762_51",
"katana"

This comment has been minimized.

Copy link
@KA101

KA101 Aug 15, 2014

Contributor

Certainly a scabbard to go with?

"type" : "start_location",
"ident" : "lab",
"name" : "science lab",
"target" : "lab"

This comment has been minimized.

Copy link
@KA101

KA101 Aug 15, 2014

Contributor

Labs are randomly generated, so currently you'd need to do a lot of work to start in a particular lab spot. If these aren't working out, try lab_core or lab_stairs.

This comment has been minimized.

Copy link
@BeigeSand

BeigeSand Aug 15, 2014

Author Contributor

I'd be happy to start in ANY lab spot. Start location currently only operates on z-level 0, so I'm trying to find out why it can't seem to find the surface labs.

// Strategy: a third of the time, return the generic scenario. Otherwise, return a scenario,
// weighting 0 cost scenario more likely--the weight of a scenario with cost n is 2/(|n|+2),
// e.g., cost 1 is 2/3rds as likely, cost -2 is 1/2 as likely.
scenario* scenario::weighted_random()

This comment has been minimized.

Copy link
@KA101

KA101 Aug 15, 2014

Contributor

Ugh, please no scenarios showing up in Play Now! It's annoying enough bugtesting and being handed a tweaker or bionic-monster already.

This comment has been minimized.

Copy link
@BeigeSand

BeigeSand Aug 15, 2014

Author Contributor

That's just remnants from the profession. If you check newcharacter.cpp, it'll never start in any scenario besides generic.

else if (g->scen->has_flag("WIN_START")){calendar::turn += DAYS((int)ACTIVE_WORLD_OPTIONS["SEASON_LENGTH"] * 3);}
else {debugmsg("The Unicorn");}
}
else{

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member
  1. bracket style
  2. this is getting big, please extract it to a "determine_starting_season()" method.
tab += set_traits (w, this, points, max_trait_points);
break;
case 2:
case 0:

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

uh... why put case 0 in the middle?

@@ -441,7 +449,7 @@ bool player::create(character_type type, std::string tempname)
else if (has_trait("HYPEROPIC")) {
prof_items.push_back("glasses_reading");
}

//LOOK HERE FOR ITEM STUFF!

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

What does this even mean?
If you want to organize things, see if you can pull out the coherent bits to helper methods, "section comments" tend to just add clutter and go out of sync with the code..

tab_captions.push_back(_("TRAITS"));
tab_captions.push_back(_("PROFESSION"));
tab_captions.push_back(_("SKILLS"));
tab_captions.push_back(_("SKILLS"));

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

I know it seems minor, but check over your diffs for random stuff like added whitespace that isn't supposed to be there and clean it up.

}

// Sort professions by name.
// profession_display_sort() keeps "unemployed" at the top.
std::sort(sorted_profs.begin(), sorted_profs.end(), profession_display_sort);
if (sorted_profs.size() > 1){

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

What's this check for? I'm pretty sure sort is safe to call on an empty or singleton container, and it just clutters up the code.

@@ -1123,7 +1136,15 @@ int set_profession(WINDOW *w, player *u, int &points)
mvwprintz(w, 5 + i - iStartPos, 2, col,
sorted_profs[i]->gender_appropriate_name(u->male).c_str());
}

}
else{

This comment has been minimized.

Copy link
@kevingranade

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

The ??? was meant to indicate, "this also does some kind of workaround for sorted_profs.size() <= 1 where it shouldn't need to". Sorry for being unclear.

@@ -1,5 +1,6 @@
#include "player.h"
#include "profession.h"
#include "scenario.h"

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

This is unnecessary.

@@ -23,6 +23,7 @@ class game;
struct trap;
class mission;
class profession;
class scenario;

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 16, 2014

Member

This is unnecessary too.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2014

Alright, thanks for pointing those out kevin, as you can tell my coding is less than fantastic. I think I got most of it though.

{
"type": "scenario",
"ident" : "mutant",
"name" : "Expirement",

This comment has been minimized.

Copy link
@KA101

KA101 Aug 16, 2014

Contributor

Intentional?

This comment has been minimized.

Copy link
@BeigeSand

BeigeSand Aug 16, 2014

Author Contributor

Yes, SUR_START should be doing the same thing as surrounded start.

This comment has been minimized.

Copy link
@KA101

KA101 Aug 16, 2014

Contributor

Uh, no, the typo. Expirement -> Experiment. But given the situation it works as a pun, leastwise in English. ;-)

This comment has been minimized.

Copy link
@BeigeSand

BeigeSand Aug 16, 2014

Author Contributor

Oh, no that was just me sucking ass at my native language...

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2014

Functionally everything seems to work fine, got people spawning in labs. (Anticipation Intensifies)

@BeigeSand BeigeSand changed the title Scenarios[CR] Scenarios[READY] Aug 17, 2014


for (int i = iStartPos; i < iStartPos + ((iContentHeight > sorted_profs.size()) ?
sorted_profs.size() : iContentHeight); i++)
{

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 18, 2014

Member

Please fix your brackets in here, also the indentation is crazy, making me suspect tabs.

cur_id = 0;
}
} else if (action == "UP") {
cur_id--;
if (cur_id < 0) {
cur_id = profession::count() - 1;
//cur_id = profession::count() - 1;

This comment has been minimized.

Copy link
@kevingranade

kevingranade Aug 18, 2014

Member

Commented out code is bad mojo.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2014

Sorry, though I had gotten rid of all the comment code. And are tabs bad? Haven't worked on much community code so I apologize if I'm somewhat lacking in code etiquette.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2014

Tab characters insert a different amount of space (whilst still only being one character, for editing purposes) depending on the software/settings viewing them. So what looks good to one of us may be illegibly formatted to another.

So we recommend you use an editor that lets you reprogram the Tab key to instead insert multiple spaces. N++ does, under Settings->Preferences->Tab Settings: Replace by space, and mine is set to 2 spaces. Thus TabTab == SpaceSpaceSpaceSpace, and you can confirm that by pushing Left four times, getting the cursor back where it was.

@BeigeSand BeigeSand closed this Aug 18, 2014

@BeigeSand BeigeSand reopened this Aug 18, 2014

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2014

Crap, hate my phone >.<

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2014

The scenario class seems to duplicate most (or all?) members of the profession class.

Why does it not inherit from the profession class (class scenario : public profession) or simply contain a profession as member?

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2014

Originally I didn't because I thought I would have to rewrite most of the methods to handle game instead of player. As it got farther along and it became clear that was less of an issue, it was mostly because I had already renamed and changed everything over.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2014

OK, so I'm registering a rewrite request from @BevapDin there--or is this actually ready for mergetest?

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2014

Not a request, I just noticed there are some overlapping functions (gender based items/name,description). As long as it does what it's supposed to do, everything is fine.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2014

Something I had not thought of when I was originally working on this, would this break compatibility with older saves? If so, I should probably fix that.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2014

would this break compatibility with older saves?

Your changes do not store anything in the save nor does it read anything, so it's probably find.

You might (once the scenario is importation after the start) want to store the ident of the scenario. In that case make sure that an unknown entry in the save (or no entry at all) are handled properly (see player::deserialize how it loads the profession). Basically fall back to a default scenario.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2014

Waiting on save-handling, then.

@BeigeSand

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2014

Fixed stuff, recovered not broken stuff, then fixed it again. Oops.

NM it's still broken :x

@KA101 KA101 self-assigned this Aug 21, 2014

@@ -364,7 +364,7 @@ class player : public Character, public JsonSerializer, public JsonDeserializer
void ma_onblock_effects();
/** Fires all get hit-triggered martial arts events */
void ma_ongethit_effects();

void empty_traits();

This comment has been minimized.

Copy link
@KA101

KA101 Aug 21, 2014

Contributor

Impressive merge conflict thanks to reformatting. Little more careful next time, hm?

This comment has been minimized.

Copy link
@BeigeSand

BeigeSand Aug 24, 2014

Author Contributor

Sorry, still fairly confused on how that particular line is breaking anything. It seems to be formatted just like everything else.

This comment has been minimized.

Copy link
@KA101

KA101 Aug 24, 2014

Contributor

That line didn't break anything, but about half the file got duplicated with about 2 spaces different indentation. So yeah, I've kept the fixed branch around rather than having to re-fix it.

if (g->scen->has_flag("SUM_START")){initial_season = 2;}
if (g->scen->has_flag("SPR_START")){initial_season = 1;}
if (g->scen->has_flag("AUT_START")){initial_season = 3;}
if (g->scen->has_flag("WIN_START")){initial_season = 0;}

This comment has been minimized.

Copy link
@KA101

KA101 Aug 21, 2014

Contributor

These may need redone. Will check.

scen_gender_items = sorted_scens[cur_id]->items_female();
}
scen_items.insert( scen_items.end(), scen_gender_items.begin(), scen_gender_items.end() );
int line_offset = 1;

This comment has been minimized.

Copy link
@KA101

KA101 Aug 22, 2014

Contributor

Unused variable here. Not sure if it's needed somewhere or just copied.

@KA101

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2014

NM it's still broken :x

Got a bug that needs hunted too so I'll shelve this one and take care of that.

@KA101 KA101 removed their assignment Aug 22, 2014

@BeigeSand BeigeSand closed this Aug 25, 2014

@BeigeSand BeigeSand reopened this Aug 25, 2014

@BeigeSand BeigeSand closed this Aug 25, 2014

@BeigeSand BeigeSand reopened this Aug 25, 2014

@KA101 KA101 self-assigned this Aug 25, 2014

BeigeSand added some commits Aug 25, 2014

@moist-zombie

This comment has been minimized.

Copy link

commented Aug 25, 2014

Keep hammering on this, Beige! :) Having this finally implemented will enhance immersion and storytelling quite a bit :>

@KA101

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2014

We're working on it.

@KA101 KA101 merged commit bbd8263 into CleverRaven:master Aug 25, 2014

1 check failed

default Unmergeable pull request.

@BeigeSand BeigeSand deleted the BeigeSand:scenario branch Sep 8, 2014

@kevingranade kevingranade changed the title Scenarios[READY] Scenarios Sep 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.