-
Notifications
You must be signed in to change notification settings - Fork 177
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
Story Arcs Basic Architecture #2997
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2997 +/- ##
============================================
- Coverage 10.65% 10.41% -0.25%
- Complexity 5535 5536 +1
============================================
Files 839 882 +43
Lines 114352 117023 +2671
Branches 17199 17569 +370
============================================
+ Hits 12187 12189 +2
- Misses 100934 103604 +2670
+ Partials 1231 1230 -1 ☔ View full report in Codecov by Sentry. |
This sounds cool. If I may make a suggestion, having a static pool of units for the opponent, allies, neutral/civ and neutral aggressive teams if present in the story would be awesome. I think I saw something about static forces for stratcon in the pulse in the past few months but I can't remember what it was about. Another suggestion would be to add a way to import unit lists to populate a static pool. That way canon unit lists could be brought in as an opponent. If it has a fluff section that can optionally list canon historical deployment areas it would also help with a petty issue I have with contracts in that I don't know what would be a reasonably canon unit I could name the bot opponent. Being able to import a unit with it's unitlist within it's inception and disbandment dates would also be useful for that, especially if it's capable of importing partial units like a company or regiment out of a multi regiment unit. This second suggestion is not a big issue and seems like a complicated suggestion on my part though. |
Thanks for your thoughts, @Duuvian. By static, I was referring to the ability for the story arc creator to specify a specific set of OpFor units, rather than using one of our random force generating methods. It has the upside of creating more realistic and balanced OpFors, but has the downside of not necessarily being balanced against the player's forces (although I can imagine ways to enforce this by limiting the units a player can deploy). I also imagine the possibility of mixing the two types in cases where the scenario requires a specific subset of units (say a commanding officer). I think what you are talking about is the ability to track the overall OpFor forces across multiple scenarios, so you can account for unit losses and the like. I agree that this would be a nice feature, but I doubt it will be part of this PR. The goals here are more basic, to give us an architecture for this kind of play that we can build upon. Now on the idea of canon unit lists, we have talked about the possibility of including this kind of information (specifically, what units are garrisoning what planets) in our universe data. That wouldn't be part of this PR but it is something that if implemented, the story arcs (and other things like AtB/stratcon) could make use of. There is no feasible way that anyone is going to generate full TO&Es that are dynamic over time, but we could set up some basic parameters like average skill rating, tech level, percent clan tech, etc. that can help inform a random force generator that can then produce more realistic units. So you could say something like "give me a heavy mech lance from the 9th Marik Militia" and you get something that makes sense for the canon unit and the time period. But that is a very different feature than what I am doing here. |
Ok, I think its time for an update. I have made a lot of progress. The following changes have been made:
What I still envision doing before the PR is ready:
One nice thing about this PR is that it has a fairly light touch with existing code. Mostly I had to add some code to get story arcs in and loaded, but otherwise even if story arcs are full of bugs, it has a low chance of affecting other areas of MHQ. |
This pull request introduces 1 alert when merging f348bde into 69ee624 - view on LGTM.com new alerts:
|
One other thing I forgot in the previous list:
|
This pull request introduces 1 alert when merging fda1bcf into e9e52e5 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 81b266b into e9e52e5 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 8482536 into e9e52e5 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 494855a into e9e52e5 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 76a06ca into e9e52e5 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging c15d31c into b2c325a - view on LGTM.com new alerts:
|
…te to field I took id out of the storypoint attribute and made it a separate field and replaced it with a name attribute. The name variable is not used for anything but will make it easier for humans to read the XML file and (eventually) the editor.
…ute to field identical to previous commit but for Personality rather than StoryPoint
This pull request introduces 1 alert when merging 0e2a809 into 237f456 - view on LGTM.com new alerts:
|
I was previously just loading all of the possible story arcs for the selector. However, because campaign and version were null at this point, I was passing on both of these null objects into other loaders which was super dangerous and eventually led to problems with loading BotForces in scenarios. So now I load a StoryArcStub instead which just has basic descriptive information and use the selected stub to load the actual story arc once it is selected and after a campaign has been loaded. I also added a version attribute to the top level of the StoryArc xml file to deal with Version.
This pull request introduces 1 alert when merging dc93ae1 into 4a26810 - view on LGTM.com new alerts:
|
…atusStoryPoint I also added a PersonStatusChangedEvent which specifically triggers when a person's status changes. This will limit the number of times the handler in StoryArc gets called.
//endregion Constructors | ||
|
||
//region Getter/Setters | ||
public void setTitle(String t) { this.title = t; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise I believe the style guide wants these to be 3 lines each: https://github.com/MegaMek/megamek/wiki/MegaMek-Coding-Style-Guide#braces
JPanel buttonPanel = new JPanel(new BorderLayout()); | ||
|
||
doneButton = new JButton("Done"); | ||
//doneButton.setMnemonic('o'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good!
This PR adds a major new feature, Story Arcs. I won't go over what story arcs are here as that is covered extensively on the Project Details Page for the GH project. Here I will discuss relevant information for reviewers.
This PR adds the underlying architecture for Story Arcs, and provides a test story arc named "Young Wolves" that players can use to try out the concept. It does not contain any ability to create story arcs apart from writing literal XML code. That is the next major planned PR in the project.
I have tried to design the architecture of story arcs so that additional features are relatively easy to implement by simply extending the abstract classes of
StoryPoint
andStoryTrigger
to new classes that can do particular things. In particular, as we develop story arcs further, we will likely see lots of requests to expand the use ofStoryTriggers
which can be used to make changes to the underlying campaign based on a certain result from aStoryPoint
. Currently, I only built the triggers necessary for the example story arc, and one could imagine all kinds of other things that could be added.StoryPoints
provide much more coverage of the kinds of things that are needed, but I can see the need to expand the number of "Check" StoryPoints which check a condition of something in the campaign.Although this PR changes/adds a lot of files, most of the work is contained within story arcs and should not affect base code. Any additional new features that I needed to get story arcs up and off the ground that involved significant changes to the base MHQ code have already been done as separate PRs and merged into master. Therefore, this code should touch existing code relatively lightly and any errors remaining will likely just mess up story arcs, not MHQ more broadly.
Given the extensiveness of the code, I am holding off on writing tests for the moment as that would delay this PR even further. On the project page, I do list adding tests for story arcs as one of the future goals, but it is prioritized lower than getting out an editor.