Ruleset framework#119
Conversation
I say probably, because I can't test it on this computer - I was unable to replicate the bug in the first place. At any rate, players whose turn it is not should not be able to end the turn. I did not want to do the same thing for CommandMessages until I am sure that there are none that are intended to be allowed on the enemy's turn. Because I know I'm going to get a comment about it, there are some of the bugs with this that aren't going to be possible to fix server-side without a moderate rewrite of the netcode, and either that or the client-side fix will take more time than I have tonight.
…sanity Probably fixed skip turn bug (See below)
- Marth mugshot updated to the subreddit flair - Sudden death now works as intended. As a side note, the fix also necessitated "unlocking" max hp, leading to the possibility of other effects modifiying it in the future. (Skill that reduces opponent's max HP?) - The village now has 10 Avo instead of 10 Def, as was intended. When we actually settle on how we want it balanced, we can revisit this.
See Ray's suggestion.
Forgot to change the comment back
Changed Marth mugshot to another different one.
See title
-Mugshot changed back to the subreddit mugshot
Some of the framework for the new rulesets. There is a new ruleset called Tournament Rules now. -Sets the minimum level for all units to 10 and the max to 20 -Adjusts exp to compensate. The shop is next, but the way it's set up, it's going to take some time to architect a good way to do it. I'm probably going to work on it more tomorrow.
|
I was hoping for this to be a GUI expansion of FEServer rather than just a Modifier but oh well. If I'm examining the code correctly, it seems that the settings are hardcoded all the way to the point of even readjusting the EXP via a hardcoded value instead of simply doing the math needed to auto-level to the desired Min LVL and deduct the appropriate amount of EXP. Mind you that the setting can be easily defeated without a Min Units setting to compliment it. Check the Reddit post linked in #111 for a good idea of how these should be fleshed out. Regardless, there should be a new config file that these settings are written to for end-user accessibility and adjustment. I get that this is just a framework, but having values hardcoded is the opposite of what we're aiming for. |
|
We'll get there. Its just so I can worry about one part at a time. The first part is getting the game to recognize variables rather than hard coded numbers as it was before. Step 2 is to pass it from elsewhere, such as a gui. If you dont break it down it becomes a monolithic mess. And doing it the way you suggest makes matters more complicated for the end user when the GUI is built. Who has the time to sit around and compute how much exp would be absorbed by a minimum level? It's unintuitive and clunky. The "squeeze exp" function is the way it was always, I only changed where it squeezed to. Quite frankly, a lot of these complaints (aside from the "why are you not done yet", which is a time issue) have been improved, assuming I'm reading your remark right. -------- Original message -------- I was hoping for this to be a GUI expansion of FEServer rather than just a Modifier but oh well. If I'm examining the code correctly, it seems that the settings are hardcoded all the way to the point of even readjusting the EXP via a hardcoded value instead of simply doing the math needed to auto-level to the desired Min Lvl and deduct the appropriate amount of EXP. Regardless, there should be a new config file that these settings are written to for end-user accessibility and adjustment. I get that this is just a framework, but having values hardcoded is the opposite of what we're aiming for. — |
I do not recall saying that the end user must / should manually calculate the EXP cost of the minimum level. I feel you misread something I said. What I was trying to say, was that I noticed that right now, you have both the minimum level and remaining exp hardcoded - as in you manually did the math. Instead, once a minimum level is set, the game (not the player) should be able to automatically level the units to the set minimum and deduct the appropriate amount of EXP from whatever the Starting BEXP setting (once implemented) is set to. In other words, right now, it seems that you're doing the math for it. Setting both the level and exp, when instead, once a minimum level is defined, the game should be able to, on it's own, make the necessary adjustments by leveling the units to the min level and paying the required amount of EXP on it's own. (i.e. If Min LVL is set to 10, then game should then be able to deduct the necessary EXP on its own from whatever the assigned Starting EXP is) Alternatively, we can simplify this setting by simply having the "Fight!" button greyed out / disabled until all the units are manually set to the user defined required minimum. (Perhaps with a message stating that "Fight!" can't be selected until all units are at least X level) |
|
Ok, I did misread that. I still disagree, but mostly because it's a pedantic distinction anyhow. I can imagine a number of issues with the other way of doing it where the team's exp ends up negative with even a little missed oversight, but at the end of the day, it doesn't really matter - both are fine. For now, for my sanity, it's staying this way, but it doesn't have to when I get everything worked out. See my comment on #111 (comment) for my plan on the matter. |
So, this is the shop limiting framework. What it doesn't do: -Give a GUI indication. Debugging this took longer than I wanted it to. I'll do it over the next few days, unless someone else would rather do it. -Allow for classes to weapons to be limited with a limit for each weapon. As it stands, each weapon can be given a limit, but you can't, say, limit a team to two "killer" weapons. That would require more formal weapon classes. Not to say I don't want to, but we'll have to figure out how to structure that. -GUI on the server side. It's still just in it's modifier. -Truly faithful to the rules as prescribed on the wiki: as a demonstration, I left each killer weapon with one buyable. What works: -Limits set on the rules server limit the purchases that the player can make. -Units that are removed from the team in blind pick "restock" the store with the items they lost when switched out. -Items have no limit by default. The inventory system utilizes a Singleton pattern, which is needed because the ShopMenu is initialized again every time the unit build screen is opened, so the data can't be stored there.
|
New commit: So, this is the shop limiting framework. What it doesn't do: What works: The inventory system utilizes a Singleton pattern, which is needed because the ShopMenu is initialized again every time the unit build screen is opened, so the data can't be stored there. |
| * destroys the current instance of the inventory. To be called when the battle starts to reset | ||
| * it for the next game. | ||
| */ | ||
| public void DestroyInstance(){ |
There was a problem hiding this comment.
If you feel the need to destroy a thing and build a new one, then singleton is the wrong pattern.
There was a problem hiding this comment.
It's that or some form of resetting it. The only time it's being destroyed is when the game starts in order to re-initialize it so the inventory restarts if multiple games are played on the same server. Otherwise, it works fine. I'd argue those two things are the same, name aside.
There was a problem hiding this comment.
A new TeamBuilderStage is initialized once per team-creation-phase, the shop inventory becomes relevant precisely when the TeamBuilderStage is created, and the shop inventory ceases to be relevant exactly when the TeamBuilderStage ceases to be relevant. And, besides it makes sense that the TeamBuilder would keep track of all limitations related to building a team, including the shop inventory.
So, I'd make an instance of ShopInventory a member of TeamBuilderStage, that instance being created in TeamBuilderStage's constructor and passed down through each new UnitBuilderStage to its ShopMenu whenever TeamBuilderStage creates a new UnitBuilderStage
And then I look closer, and a ShopInventory is a member of TeamBuilderStage, and yet UnitBuilderStage/ShopMenu still asks a global for a ShopInventory, instead of asking TeamBuilderStage. How did that come to be?
There is absolutely no requirements for interface compatibility here. ^(Well, the map and team files should remain loadable, but that's disjoint to this.) You are allowed to add or change method parameters.
There was a problem hiding this comment.
That's not a bad idea; I didn't see that there was only one TeamBuilderStage. Still trying to get used to this codebase. Nothing in software is harder than inheriting code you didn't write. :D
I'll refactor that tomorrow evening, it's a bit late for it tonight.
True, this does become more of an issue once we allow hosts to tweak the starting EXP too. Automating it might prove too complicated; though I have a few ideas: A.) We forego automated leveling to the specified minimum, and simply disable the "Fight!" button until the player satisfies both the minimum unit and level requirements (the bottom portion of the screen during the Team Builder should mention that the button is disabled until X conditions are met). Though I can't imagine a way to bypass this, there should also be a sanity check in place just in case some way, some how, it can be fast fingered. There should also be a sanity check to ensure that the EXP costs for the set minimum can be met prior to hosting. OR B.) More Sanity Checks. Have FEServer check in real-time (or at least each time the values have changed) that the combined settings won't result in a scenario where negative EXP is possible. If the check fails, the host is instructed to check their settings, make the appropriate changes, and try again.
@Purplemandown There's two ways I can see this being addressed with relative ease: Method A.) We edit Method B.) Dirty / lazy method. We can detect weapon sub-classes by polling their existing stats from either
|
|
Currently, "Brave" (in addition to the dark magic effects) is defined in Weapon::getTriggers. If Brave is to be extracted to Bows shouldn't count as Slayer weapons unless they're effective against something other than fliers. Just checking the Effective field for non-empty wouldn't be able to respect that. |
Good point, perhaps have it ignore the Flier value on weapons classed as Bows and Crossbows when checking for Slayer weapons? |
Followed Ray's suggestion on factoring.
|
This commit is just refactoring as per @rayrobdod 's suggestion. It isn't weapon groups yet, I'll get that done either this week or weekend, if not done already. |
|
Are you ready for a merge, then? |
Weapon classes work now. They read their classes from the (currently) last column in the weapon txt file, with the exception of vulnerary/concoction/elixr, which have been hard-coded (for some time, at any rate they're in the Healing class), and Rise (also hard coded, it is not in a class). The classes are Killer, Slayer, Reaver, Brave, Legend, Siege, and Healing as of now.
|
Well, given that this wasn't merged (no opinion as to whether or not that's the right call, I could see it going either way), I did some more on it. Weapon classes work now. They read their classes from the (currently) last column in the weapon txt file, with the exception of vulnerary/concoction/elixr, which have been hard-coded (for some time, at any rate they're in the Healing class), and Rise (also hard coded, it is not in a class). The classes are Killer, Slayer, Reaver, Brave, Legend, Siege, and Healing as of now. I tested them a little bit, but some more thorough testing is probably in order. Also, this doesn't address the necessary GUI component in the shop. I took a look at how it's set up, but I can't make heads or tails of it. As for @eliatlarge, we should probably hold of on merging until someone gets the shop graphics to update with this (I was thinking just gray out the text of anything that is out of stock), but I don't know how long that will take. |
|
Is the Regalia/Rapier/None lord weapon option in scope? Because "Regalia" and "Rapier" could be a useful category when that becomes a thing. It would be easy enough to do that later when that option is focused upon if you're not focusing on that now. There's no precedent (in this game; for everything else yes, but in this game no) for greying out disabled options. Expensive items don't grey out when funds are low, for instance. And either way, indicating that a category of weapons are linked would be a good thing. Maybe something like "Reaver: 2" (or whatever the category of the currently selected item is, and updated as items are bought) under the shop menu, similar to the "EXP: 5000" under the Level Up/Level Down buttons. When a team is loaded, the new team's stuff is not removed from the shop inventory or restricted due to shop inventory limitations. |
|
@Purplemandown I agree that we should hold off on merging until this comes a long a bit further. Also, how do we handle weapons with multiple attributes? Noticed you added @rayrobdod I agree that the weapons for the Lords should have categories as well. I'm a little torn on if the wording should be @Purplemandown @rayrobdod Greying out "sold out" items as well as things player have insufficient Gold / EXP for is a wonderful idea. We should definitely implement this.
We should do what it currently does when someone loads a team made with the Veterans or Treasury modifiers, parse the team from top to bottom, fulfilling as much as possible, then omitting things once limits are reached. There should also be a notification to alert the player that the team has been modified / needs adjustments. An alternative would be to "grey out" the |
|
Multiple weapon classes was something I hadnt thought of. Thatll be pretty easy-I can do it tonight or tomorrow night. The loading teams is something else I hadnt thought of. That onell likely be a bit harder. -------- Original message -------- @Purplemandown I agree that we should hold off on merging until this comes a long a bit further. Also, how do we handle weapons with multiple attributes? Noticed you added Throw as an attribute, but Gradivus is both a throwing weapon and a legendary. Sol Katti's stats for example make it a Regalia, Killer, and Slayer all rolled into one. We need a solution for having multiple attributes / values anyways once we add weapons like the Rapier which are effective against both Cavalry and Armor. That being said, to make it less ambiguous, Mount should be reworded as Cavalry @rayrobdod I agree that the weapons for the Lords should have categories as well. I'm a little torn on if the wording should be Regalia / Personal / None OR Endgame / Starter / None only because some parts of the community use the terms Regalia and Legendary interchangeably. @Purplemandown @rayrobdod Greying out "sold out" items as well as things player have insufficient Gold / EXP for is a wonderful idea. We should definitely implement this. When a team is loaded, the new team's stuff is not removed from the shop inventory or restricted due to shop inventory limitations. We should do what it currently does when someone loads a team made with the Veterans or Treasury modifiers, parse the team from top to bottom, fulfilling as much as possible, then omitting things once limits are reached. There should also be a notification to alert the player that the team has been modified / needs adjustments. An alternative would be to "grey out" the Fight! button until the team is legal. — |
|
I can do the shop greyed-out graphics; the thing is that since Eli does squash merging, and squashing tends to remove unimportant details like a secondary author, I'd selfishly prefer it not be the same merge. |
|
Oh, this got done? Nice! Thanks for taking care of it, ive been really busy at work and with getting ready to move these past few weeks. -------- Original message -------- I can do the shop greyed-out graphics; the thing is that since Eli does squash merging, and squashing tends to remove unimportant details like a secondary author, I'd selfishly prefer it not be the same merge. — |
|
I'll leave out the squash merge, I just don't like the interface being filled up with multiple commits. I'm completely fine with merging without squashing. |
ghost
mentioned this pull request
|
Did I really not merge this? @SapphireTactician , was there anything wrong with this req or was I just being lazy? |
|
@eliatlarge I thought we were waiting on @Purplemandown and @rayrobdod to flesh it out a bit more before I do any real extensive testing on it (meanwhile I believe they were waiting on us to finish with the graphics engine overhaul before moving forward - creating a standstill) - especially because some of the rules are hardcoded with no way for the end user to modify them (I'd rather not have any forced options in a public build). |
|
oh, and then I think we decided to move to TF and implement this framework there, iirc. |
3 participants
Some of the framework for the new rulesets. There is a new ruleset called Tournament Rules now. It isn't dynamic, but
-Sets the minimum level for all units to 10 and the max to 20
-Adjusts exp to compensate.
The shop is next, but the way it's set up, it's going to take some time to architect a good way to do it. I'm probably going to work on it more tomorrow.
I understand if this means we want to wait until that's done, but I figure it doesn't break anything, and stepping forward in increments is better than nothing, particularly on a dev build.