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

Story Arcs Basic Architecture #2997

Merged
merged 378 commits into from
Apr 22, 2024
Merged

Conversation

AaronGullickson
Copy link
Member

@AaronGullickson AaronGullickson commented Dec 11, 2021

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 and StoryTrigger 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 of StoryTriggers which can be used to make changes to the underlying campaign based on a certain result from a StoryPoint. 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.

@Windchild292 Windchild292 added (RFE) Enhancement Requests for Enhancement, new features or implementations Major This will require major changes across the project. labels Dec 11, 2021
@codecov
Copy link

codecov bot commented Dec 12, 2021

Codecov Report

Attention: Patch coverage is 0% with 2692 lines in your changes are missing coverage. Please review.

Project coverage is 10.41%. Comparing base (e7d4f4a) to head (4293efe).
Report is 9 commits behind head on master.

❗ Current head 4293efe differs from pull request most recent head f22b749. Consider uploading reports for the commit f22b749 to get more accurate results

Files Patch % Lines
...HQ/src/mekhq/gui/dialog/CreateCharacterDialog.java 0.00% 835 Missing ⚠️
MekHQ/src/mekhq/campaign/storyarc/StoryArc.java 0.00% 241 Missing ⚠️
...storyarc/storypoint/CreateCharacterStoryPoint.java 0.00% 157 Missing ⚠️
MekHQ/src/mekhq/campaign/storyarc/StoryPoint.java 0.00% 108 Missing ⚠️
MekHQ/src/mekhq/gui/dialog/StoryChoiceDialog.java 0.00% 79 Missing ⚠️
...ekHQ/src/mekhq/campaign/storyarc/StoryArcStub.java 0.00% 76 Missing ⚠️
...mpaign/storyarc/storypoint/ScenarioStoryPoint.java 0.00% 71 Missing ⚠️
...toryarc/storytrigger/ChangePersonStoryTrigger.java 0.00% 68 Missing ⚠️
...ampaign/storyarc/storypoint/MissionStoryPoint.java 0.00% 61 Missing ⚠️
MekHQ/src/mekhq/campaign/storyarc/Personality.java 0.00% 45 Missing ⚠️
... and 48 more
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.
📢 Have feedback on the report? Share it here.

@Duuvian
Copy link

Duuvian commented Dec 13, 2021

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.

@AaronGullickson
Copy link
Member Author

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.

@AaronGullickson
Copy link
Member Author

Ok, I think its time for an update. I have made a lot of progress. The following changes have been made:

  1. I have added a StoryNarrativeEvent which pops up a dialog with narrative information for the player.
  2. I have added a StoryChoiceEvent which pops up a dialog which asks the user to make a choice which can then affect the StoryOutcome.
  3. I have added StoryPortrait and StorySplash objects which extend AbstractIcon and contain story related portraits (used for Personalities below) and splash images. These can both be added to popup dialogs to enhance the narrative immersion.
  4. I have made it possible with some DirectoryItem-fu for creators to keep a data subdirectory in their story arc and have story arc specific portrait and splash images loaded from there.
  5. I have added a Personality object that can be associated with any StoryEvent. The main purpose of this object is that it has a portrait which is applied to any popup dialog from that StoryEvent. This can be used to make StoryNarrativeEvents and StoryChoiceEvents appear like they are being delivered by a person. Given that this works well, I will not be creating a separate ShoutStoryEvent. I will also probably push back the full rpg style dialog to a future enhancement as the basic functionality of this already provided with the StoryChoiceEvent tied to a Personality.
  6. I have done some more work on the "Young Wolves" story arc that I am building to test out the full capabilities of story arcs. Natasha Kerensky is in the game now, y'all.

What I still envision doing before the PR is ready:

  1. A TravelStoryEvent as described above.
  2. I am considering adding a CreateCharacterEvent.java that will give players an XP pool they can use to create their own ego character for the story arc. Typically this would be the first event in a campaign if it allows for it, although I could imagine other uses.
  3. Make scenarios better. This is the big one described in the original post. It might actually make more sense to do this in a separate PR as my idea is that the improvements to scenarios would not be specific to Story Arcs, but if I did it that way, I would probably not release this PR until that one was ready.
  4. Finish up my demo story arc.
  5. General code cleanup. I have been trying to document well and follow norms as I go, but I know that some touchup is necessary (as the nagging checks here on GH are also reminding me).

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.

@lgtm-com
Copy link

lgtm-com bot commented Dec 14, 2021

This pull request introduces 1 alert when merging f348bde into 69ee624 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@AaronGullickson
Copy link
Member Author

One other thing I forgot in the previous list:

  • A GUI element on the Command Center tab that lists current story arc objectives from all active StoryEvents

@lgtm-com
Copy link

lgtm-com bot commented Dec 16, 2021

This pull request introduces 1 alert when merging fda1bcf into e9e52e5 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Dec 18, 2021

This pull request introduces 1 alert when merging 81b266b into e9e52e5 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Dec 18, 2021

This pull request introduces 1 alert when merging 8482536 into e9e52e5 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Dec 18, 2021

This pull request introduces 1 alert when merging 494855a into e9e52e5 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Dec 18, 2021

This pull request introduces 1 alert when merging 76a06ca into e9e52e5 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

@lgtm-com
Copy link

lgtm-com bot commented Dec 22, 2021

This pull request introduces 1 alert when merging c15d31c into b2c325a - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

…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
@lgtm-com
Copy link

lgtm-com bot commented Dec 28, 2021

This pull request introduces 1 alert when merging 0e2a809 into 237f456 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

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.
@lgtm-com
Copy link

lgtm-com bot commented Dec 29, 2021

This pull request introduces 1 alert when merging dc93ae1 into 4a26810 - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

…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.
@Sleet01 Sleet01 self-requested a review April 11, 2024 05:42
//endregion Constructors

//region Getter/Setters
public void setTitle(String t) { this.title = t; }
Copy link
Collaborator

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');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be removed.

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good!

@AaronGullickson AaronGullickson marked this pull request as ready for review April 14, 2024 17:50
@AaronGullickson AaronGullickson merged commit 049be09 into MegaMek:master Apr 22, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Major This will require major changes across the project. (RFE) Enhancement Requests for Enhancement, new features or implementations
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants