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

Ordnance Tubes card management #15

Open
fab7431 opened this issue Nov 21, 2015 · 14 comments
Open

Ordnance Tubes card management #15

fab7431 opened this issue Nov 21, 2015 · 14 comments

Comments

@fab7431
Copy link
Collaborator

fab7431 commented Nov 21, 2015

Hi all,

How to do thing to handle the Ordnance Tubes card ?
I start thinking about it and for the time being I see two options :
1/ We assign Torpedoes and Missiles to the Hardpoint slot.
{"name":"cr90corvettefore","points":58,"ship":"cr90corvette","multisection_id":0,"upgrades":{"hardpoint":["assaultmissiles"]}}

2/ This card opens a torpedoes or missile slot to the ship :
{"name":"cr90corvettefore","points":65,"ship":"cr90corvette","multisection_id":0,"upgrades":{"missile":["assaultmissiles"],"hardpoint":["singleturbolasers"]}},

From a development stand point, I would go with the 1st option but from the xws specs the second one seems to be better.

Perhaps there are some others options. Just let me know what you think.

bye
Fab

@geordanr
Copy link
Collaborator

I'm inclined to go with the first option as well.

@fab7431
Copy link
Collaborator Author

fab7431 commented Nov 21, 2015

In fact more I think about that the more I think option 2 is the best one.

@voidstate
Copy link
Collaborator

I've been thinking about this and can't come up with anything better than number two. It's more descriptive and it mirrors other upgrades which add second upgrade.

The only problem I can see is if a ship in future gets a missile slot. How then would you know which ordnance is in the tubes?

@geordanr
Copy link
Collaborator

That's kind of why I was leaning towards the first option, but thinking about it more it isn't actually necessary to know if a missile is in Ordnance Tubes or in a native missile slot -- always attempt to populate fixed slots (torps/missiles) first, and any leftovers will have to go into converted hardpoint slots.

@voidstate
Copy link
Collaborator

I've been wrestling with implementing Ordnance Tubes for several weeks now (!). I've had to rewrite tons of code bacause of assumptions I'd made about slots holding a specific type of upgrade. Anyway, I'm nearly done now.

Currently, my XWS appears like option 2:

{
    "version": "0.3.0",
    "faction": "rebel",
    "pilots": [
        {
            "name": "cr90corvetteaft",
            "ship": "cr90corvette"
        },
        {
            "name": "cr90corvettefore",
            "ship": "cr90corvette",
            "upgrades": {
                "torpedo": [
                    "advprotontorpedoes"
                ],
                "missile": [
                    "homingmissiles"
                ],
                "mod": [
                    "ordnancetubes"
                ]
            }
        }
    ]
}

Is this OK with you guys?

@fab7431
Copy link
Collaborator Author

fab7431 commented Dec 17, 2015

Hi,

Sounds good to me, here is my xws :

{"description":"","faction":"rebel","pilots":[
{
    "name":"cr90corvettefore",
    "points":68,
    "ship":"cr90corvette",
    "multisection_id":0,
    "upgrades": {
        "missile":
            ["homingmissiles"],
        "hardpoint":
            ["singleturbolasers"],
        "mod":["ordnancetubes"]}
}
{
    "name":"cr90corvetteaft",
    "points":46,
    "ship":"cr90corvette",
    "multisection_id":0,
    "upgrades": {
        "torpedo":["advancedprotontorpedoes"]
                }
}

In term of development, when I assign a Torpedo for example, I specify that I fill a missile and hardpoint slot (I specified that the slot is occuped virtually), I used the same behavior with the Palpatine crew card.

Good luck !

@geordanr
Copy link
Collaborator

Whoops. Somehow I got it in my head that we were leaving the slot type the same:

{
    "pilots": [{
        "name": "cr90corvetteaft",
        "points": 40,
        "ship": "cr90corvette",
        "multisection_id": 0
    }, {
        "name": "cr90corvettefore",
        "points": 64,
        "ship": "cr90corvette",
        "upgrades": {
            "hardpoint": ["quadlasercannons", "ionpulsemissiles"],
            "mod": ["ordnancetubes"]
        },
        "multisection_id": 0
    }],
}

(I'm also having trouble granting the ability to the other half of the ship... it's basically requiring an immense rewrite)

From a gameplay/rules perspective, keeping the slot as a hardpoint is more correct; the slot type hasn't changed, but what it can accept has. Is that a concept we want to track in XWS?

@fab7431
Copy link
Collaborator Author

fab7431 commented Dec 17, 2015

Hi,
As I completed the implementation the first of december, I really don't want to change anything right now.
The voidstate comment about option 2 sounded good to me.
It seems to me more simple to understand if we open missiles and torpedoes slots than putting missiles and torpedoes in the hardpoint slots. Keeping the upgrades on their respective slot makes the xws more understandtable (I think).
Honestly, handling this card is quite an headache. So good luck to both of you.

Off topic : Do you plan to go to Celebration europe next year ? I will.

@lhayhurst
Copy link
Collaborator

sorry for late weigh-in. I'm cool with whatever you guys come up with here. sorry everyone had to rewrite a bunch of code :-(

@geordanr
Copy link
Collaborator

Seriously, Ordnance Tubes is a headache 😭

@fab7431
Copy link
Collaborator Author

fab7431 commented Dec 18, 2015

Try to think how you handled the following cards, "Mist hunter", "Emperor Palpatine" and "Heavy Scyk Interceptor", by mixing all you did with them you probably can find a solution.
If you need a little help, don't hesitate to ask, sometimes by discussing a little bit, it opens new way.
You can also check in my builder the behavior of this card.
Good luck.

@elistevens
Copy link
Owner

Been thinking about this a lot, since there are a lot of possible gotchas.

  • We need to keep the slot type, since it's possible that there will be an epic ship with normal missile or torpedo slots.
  • We need to keep the upgrade type, since it's possible that there will be an upgrade that lets a ship equip astromechs as crew (and so "crew": ["r2d2"] will be ambiguous).

I think we should mandate that when an upgrade card is going into a slot that doesn't match its normal type, we suffix the upgrade name with a dash and the upgrade type. So:

"hardpoint": ["ionpulsemissile-missile"]
"crew": ["r2d2-amd"] (in the hypothetical above)

Or, more generally:

"slot": ["name-otherupgradetype"]

I think that it reads well, and since we don't allow dashes in the canonicalized name, it's unambiguous.

I think that the -type suffix should be mandatory every time the slot doesn't match (which I think is just ordnance tubes right now), and am on the fence if it should be optional when not needed.

@fab7431
Copy link
Collaborator Author

fab7431 commented Dec 20, 2015

Hi,
I disagree going this way Eli. I think cards have to go in their respective slots, it seems to me too intricate going another way.
If we have one day a new epic ship with two hardpoint slots and a legal torpedo slot for example, if you add Ordance Tubes to that ship, you will get potentially 3 torpedo slots (2 missiles and the original 2 hardpoints), the first slot is the legal torpedo one and the next two, the ones coming with Ordnance Tubes, I don't see the point to see things differently.
The main constraint is at the builder level where we have to "link" the slots open by the Ordnance Tubes or to find a way to handle the card.
Voidstate and I implemented that way the xws export. The Heavy Scyk Interceptor card is very closed to that behaviour and we didn't change anything for it.
It is my position for the time being... but open to discussion.

@fab7431
Copy link
Collaborator Author

fab7431 commented Jan 12, 2016

So, where are we about that ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants