This project is read-only.

Multiple forks

May 1, 2013 at 6:07 AM
So, as I am fairly new to this, I am not quite sure how multiple forks work.

For example: Fumbles has a fork where he did a lot of changes, and at the same time, I have a fork where I implemented all of the 2.0 skill changes.

Since some changes are in both of ours, and they will slightly conflict, do I need to base my changes off of his fork?

Or should I just focus on the main and do things get resolved upon merge?
May 1, 2013 at 6:12 PM
Hi Xurkane,

I believe we fork from the main and let TeamRobot sort it out at the merge. However, we can (and probably should) coordinate here first so that we're not all recreating each other's work.

Regards,
Fumbles
May 1, 2013 at 7:34 PM
Yeah -- coordinating to avoid overlapping effort would be ideal, but ultimately it falls on whoever does the merge (me right now) to sort it out in the end. Worst case: I might reject a pull request and tell you to pull latest from main branch and merge your own code with another user's changes.

I have merged latest from fumbles and xurkane -- it probably needs some testing though. Nothing super time consuming: just make a profile for each type of character (only need to do 8, since empire/republic are mirrors). Then follow the example code in the documentation for testing it -- just give the stats generated a once-over and make sure they are correct. In particular, look for any NaN -- that probably means a bad skill tree index somewhere, or something of that nature.

At the bottom of here is a comment on how to test:
https://amrswtor.codeplex.com/wikipage?title=Character%20Stat%20Code&referringTitle=Documentation

This document shows you how to write a simple html page that can load any character profile from the live site for testing:
https://amrswtor.codeplex.com/wikipage?title=Character%20Stat%20API&referringTitle=Character%20Stat%20Code


Once one of you (or anyone else who wants to give them a once-over) gives me the thumbs up on the latest code, I'll update the live website.
May 2, 2013 at 4:55 AM
So apparently I fail at testing. I swear I did what your documentation said and it just didn't seem to come out right, or rather, I couldn't get anything to happen at all. If anyone sees any problems with the skill trees data though please let me know so I can correct it.
May 2, 2013 at 3:42 PM
As I don't have a full complement of 8 advanced classes, I can only test a few of these.

Looking at the merged code, the only concern I have are these two lines in the skillFuncs.Operative section:
    incrementStat(stats, 'BuffTechBonusHealing', 0.01 * trees[0][4]);

    incrementStat(stats, 'BuffTechBonusHealing', 0.01 * trees[1][2]);
As I stated in a previous commit, these are currently bugged in game in that their benefit doesn't show up on the character sheet. I have confirmed that they do indeed work, so the skills themselves aren't broken, just the in game display of the information.

So this leads us to a philosophical discussion of whether we feel AMR should match the in game display (known bugs and all) or show the actual benefit knowing ahead of time that many people will not understand that there's a UI bug in the game itself. Personally I think the primary use case for AMR is to have confidence that the character builder matches the in game displayed stats, so I prefer leaving the skills in there, but properly commented. It's a tough call as this conflicts with the purist side of me.
May 2, 2013 at 3:51 PM
Edited May 2, 2013 at 3:53 PM
Xurkane wrote:
So apparently I fail at testing. I swear I did what your documentation said and it just didn't seem to come out right, or rather, I couldn't get anything to happen at all. If anyone sees any problems with the skill trees data though please let me know so I can correct it.
Hi,

I made this little page for testing the calculations. See if it works for you.

Fumbles
<!DOCTYPE html>
<html>
<head>
    <title></title>
    <script src="http://code.jquery.com/jquery-1.9.1.min.js"></script>
    <script src="amrswtor/CharacterCalculations/Swtor.Calc.Standalone.js"></script>
    <script type="text/javascript">

var g_profile = null;

function getProfile(profileId) {
    $.ajax({
        url: "http://swtor.askmrrobot.com/api/character/v1/" + profileId,
        type: "GET",
        dataType: "jsonp",
        success: function(profile) {
            g_profile = profile;
            renderProfile(profile);
        }
    })
}

function renderProfile(profile) {
    stats = Tr.Swtor.Calc.getProfileStats(profile);
    var html = [];
    for (var stat in stats)
        html.push(stat + ": " + stats[stat] + "<br/>");

    $("#panelDebug").html(html.join(""));
}

function doTest() {
    getProfile($("#profId")[0].value);
    return false;
}

function strToTree(skillString) {
    var trees = null;
    if (skillString != null) {
        trees = new Array();
        var parts = skillString.split('-');
        $.each(parts, function (i, val) {
            var points = new Array();
            for (var i = 0; i < val.length; i++)
                points.push(parseInt(val[i]));
            points.reverse();
            trees.push(points);
        });
    }
    return trees;
}

function sorcSkillNames() {
    var names = [
        [
            "Forceweaver",
            "Seeping Darkness",
            "Force Suffusion",
            "Lucidity",
            "Empty Body",
            "Dark Mending",
            "Sap Strength",
            "Force Bending",
            "Resurgence",
            "Efficacious Currents",
            "Dark Resilience",
            "Sith Purity",
            "Reconstruct",
            "Fadeout",
            "Conspiring Force",
            "Innervate",
            "Corrupted Barrier",
            "Life Surge",
            "Force Surge",
            "Penetrating Darkness",
            "Corrupted Speed",
            "Twisted Force",
            "Reverse Corruptions",
            "Revivification"
        ],
        [
            "Chain Shock",
            "Reserves",
            "Electric Induction",
            "Lightning Spire",
            "Convection",
            "Subversion",
            "Lightning Barrier",
            "Lightning Barrage",
            "Chain Lightning",
            "Lightning Effusion",
            "Lightning Swarm",
            "Lightning Storm",
            "Backlash",
            "Conduction",
            "Forked Lightning",
            "Suppression",
            "Electric Bindings",
            "Chaos Nexus",
            "Reverberating Force",
            "Force Haste",
            "Forked Darkness",
            "Thundering Blast"
        ],
        [
            "Calcify",
            "Sith Defiance",
            "Will of the Sith",
            "Oppressing Force",
            "Force Horrors",
            "Parasitism",
            "Disintegration",
            "Death Field",
            "Sith Efficacy",
            "Madness",
            "Haunted Dreams",
            "Corrupted Flesh",
            "Wrath",
            "Deathmark",
            "Lightning Burns",
            "Lingering Nightmares",
            "Creeping Death",
            "Devour",
            "Focal Lightning",
            "Surging Speed",
            "Shapeless Spirit",
            "Creeping Terror"
        ]
    ];
    return names;
};

function mercSkillNames() {
    var names = [
        [
            "Improved Vents",
            "Med Tech",
            "Hired Muscle",
            "Empowered Scans",
            "Surgical Precision System",
            "Heat Damping",
            "Critical Reaction",
            "Kolto Pods",
            "Kolto Residue",
            "Supercharged Gas",
            "Power Shield",
            "Powered Insulators",
            "Critical Efficiency",
            "Protective Field",
            "Reactive Armor",
            "Proactive Medicine",
            "Kolto Shell",
            "Cure Mind",
            "Warden",
            "Peacekeeper",
            "Kolto Jets",
            "Bodyguard",
            "Emergency Scan",
        ],
        [
            "Mandalorian Iron Warheads",
            "Stabilizers",
            "Ironsights",
            "Power Barrier",
            "Upgraded Arsenal",
            "Custom Enviro Suit",
            "Afterburners",
            "Tracer Missile",
            "Target Tracking",
            "Jet Escape",
            "Tracer Lock",
            "Terminal Velocity",
            "Pinning Fire",
            "Riddle",
            "Light 'Em Up",
            "Torque Boosters",
            "Barrage",
            "Power Overrides",
            "Decoy",
            "Energy Rebounder",
            "Power Launcher",
            "Heatseeker Missiles",
        ],
        [
            "Advanced Targeting",
            "System Calibrations",
            "Integrated Cardio Package",
            "Superheated Gas",
            "Sweltering Heat",
            "Gyroscopic Alignment Jets",
            "Pyro Shield",
            "Superheated Rail",
            "Incendiary Missile",
            "Infrared Sensors",
            "Suit FOE",
            "Prototype Particle Accelerator",
            "Volatile Warhead",
            "Firebug",
            "Rain of Fire",
            "Degauss",
            "Burnout",
            "Rapid Venting",
            "Power Barrels",
            "Jet Rebounder",
            "Thermal Detonator",
        ]
    ];
    return names;
};

function operSkillNames() {
    var names = [
        [
            "Incisive Action",
            "Precision Instruments",
            "Imperial Education",
            "Endorphin Rush",
            "Medical Consult",
            "Surgical Steadiness",
            "Chem-resistant Inlays",
            "Prognosis: Critical",
            "Kolto Probe",
            "Sedatives",
            "Patient Studies",
            "Medical Engineering",
            "Evasive Imperative",
            "Tox Screen",
            "Medical Therapy",
            "Surgical Probe",
            "Surgical Precision",
            "Med Shield",
            "Accomplished Doctor",
            "Surprise Surgery",
            "Durable Meds",
            "Recuperative Nanotech"
        ],
        [
            "Concealed Attacks",
            "Imperial Brew",
            "Survival Training",
            "Infiltrator",
            "Surgical Strikes",
            "Inclement Conditioning",
            "Scouting",
            "Flanking",
            "Laceration",
            "Collateral Strike",
            "Pin Down",
            "Tactical Opportunity",
            "Ghost",
            "Waylay",
            "Advanced Cloaking",
            "Culling",
            "Revitalizers",
            "Meticulously Kept Blades",
            "Jarring Strike",
            "Calculated Frenzy",
            "Shadow Operative Elite",
            "Acid Blade"
        ],
        [
            "Deadly Directive",
            "Lethality",
            "Hit and Run",
            "Slip Away",
            "Flash Powder",
            "Corrosive Microbes",
            "Lethal Injectors",
            "Corrosive Grenade",
            "Combat Stims",
            "Cut Down",
            "Lethal Purpose",
            "Adhesive Corrosives",
            "Escape Plan",
            "Lethal Dose",
            "Augmented Shields",
            "Cull",
            "License to Kill",
            "Counterstrike",
            "Devouring Microbes",
            "Lingering Toxins",
            "Toxic Regulators",
            "Fatality",
            "Quickening",
            "Weakening Blast",
        ]
    ]
    return names;
};

    var dr_b = {
        'Absorb': [0.5, 0.5, 0.5, 0.5, 0.5, 0.5],
        'Accuracy': [0.3, 0.3, 0.3, 0.3, 0.3, 0.3],
        'Defense': [0.3, 0.3, 0.3, 0.3, 0.3, 0.3],
        'Alacrity': [0.3, 0.3, 0.3, 0.3, 0.3, 0.3],
        'Crit': [0.3, 0.3, 0.3, 0.3, 0.3, 0.3],
        'CritPrimary': [0.3, 0.2, 0.2, 0.2, 0.2, 0.2],
        'Shielding': [0.5, 0.5, 0.5, 0.5, 0.5, 0.5],
        'Surge': [0.3, 0.3, 0.3, 0.3, 0.3, 0.3],
    };

    // N is the divisor of the formula and is stable for levels 1-50
    // as of 2.0, N now varies for every level 51-55

    // tank stats (absorb, defense, shield) are not complete for 51-54
    // pvp calculation has been removed as it no longer has a DR.

    var dr_N = {
        'Absorb': [9, 13.5, 18, 23, 28, 32.5],
        'Accuracy': [27.5, 34, 40.5, 47, 53.5, 60],
        'Defense': [27.5, 34, 40.5, 47, 53.5, 60],
        'Alacrity': [27.5, 34.5, 41.5, 48.5, 55.5, 62.5],
        'Crit': [22.5, 27, 31.5, 36, 40.5, 45],
        'CritPrimary': [125, 155, 185, 215, 245, 275],
        'Shielding': [16, 20.5, 25, 30, 34.5, 39],
        'Surge': [5.5, 6.5, 7.5, 9, 10, 11],
    };


    function getDiminishedStat(stat, lvl, rating, N) {

        var idx = (lvl < 50 ? 0 : lvl - 50)
        var b = dr_b[stat][idx];
        if (N === undefined)
            N = dr_N[stat][idx];
        var r = rating / Math.max(lvl, 20);
        return b * (1 - Math.pow(1 - (0.01 / b), r / (N / 50)));
    }


function setProfId(profId) {
    $("#profId")[0].value = profId;
}

    </script>
</head>
<body>

<form action="#">
    <input id="profId" size="40" label="profileId" value=""><br/>
    <input type="button" label="GenSorc" value="GenSorc" onclick="setProfId('4bf344b2-b516-4bb9-82c9-e9f4e98a13e6'); doTest(); return false">
    <input type="submit" onclick="doTest(); return false">
</form>

    This panel is where we will print out the stats for testing:
    <div id="panelDebug"></div>
</body>
</html>
May 2, 2013 at 3:53 PM
I must have missed that note in one of the commits. I would agree that if they do not show up in the in-game UI then they should likely be commented out so that people will not dispute the tool. But i would also suggest that we notate why in a comment as well so that everyone is aware who might be working on the code.

As far as testing, I believe yellowfive just wanted someone to make profiles for each AC and make sure no glaring issues showed up.

Also, Fumbles, do you have any idea why this line might have caused a bug? I cannot seem to find an issue with it:
             //incrementStat(stats, 'BuffRangedCritChance', 0.01 * trees[1][21]);
It is in the skillFuncs.Powertech section.
May 2, 2013 at 3:57 PM
Your test page worked great, thanks a bunch.
May 2, 2013 at 4:46 PM
Xurkane wrote:
I must have missed that note in one of the commits. I would agree that if they do not show up in the in-game UI then they should likely be commented out so that people will not dispute the tool. But i would also suggest that we notate why in a comment as well so that everyone is aware who might be working on the code.

As far as testing, I believe yellowfive just wanted someone to make profiles for each AC and make sure no glaring issues showed up.

Also, Fumbles, do you have any idea why this line might have caused a bug? I cannot seem to find an issue with it:
             //incrementStat(stats, 'BuffRangedCritChance', 0.01 * trees[1][21]);
It is in the skillFuncs.Powertech section.
I'm seeing that skill as "Power Loaders" which doesn't improve ranged crit. Which skill is that supposed to be?
May 2, 2013 at 4:54 PM
No you're right, it needs to be removed and its on my todo list for today but I was more curious as to why it caused an error. Also, how do I make my fork match current existing code? Or do I need to delete and recreate the fork?
May 2, 2013 at 5:42 PM
Edited May 2, 2013 at 5:44 PM
I did...

hg pull https://hg.codeplex.com/amrswtor
hg update

That should resync your fork back to the main branch, followed by...

hg summary

...should show that your local clone's parent is the main branch id.
hg summary
parent: 18:6ac5df99ee27 tip
added new stim level
branch: default
commit: (clean)
update: (current)
May 2, 2013 at 5:52 PM
I will attempt that next time. Also, I submitted a pull request for the Power Loaders and tech healing changes. Sadly, I do not have time to test my toons, but I will be going through everything tonight to see if I can find any problems.
May 4, 2013 at 6:53 PM
//incrementStat(stats, 'BuffRangedCritChance', 0.01 * trees[1][21]);
I think that I see why this caused an error... something I'll have to look at. Old profiles saved with pre-2.0 talent strings might not have enough talents, so there wasn't really an error in anything you did (although, I don't see that skill actually giving crit -- is our skill tree data out of sync with the game?).

I don't think it's worth writing code to retroactively fix old saved profiles... simply opening the skill tree editor and pressing Save solves the problem.


And Xurkane has the idea -- just a quick test on each advanced class to look for anything way off. Simply making a saved profile for a generic character of that type is sufficient. You don't even need to put on a full set of gear, just pick a few random items.
May 4, 2013 at 6:55 PM
Yellowfive wrote:
I think that I see why this caused an error... something I'll have to look at. Old profiles saved with pre-2.0 talent strings might not have enough talents, so there wasn't really an error in anything you did (although, I don't see that skill actually giving crit -- is our skill tree data out of sync with the game?).
I think I was looking at old skill trees actually, the crit definitely shouldn't be there. I'll be making generic profiles for each AC here in a little bit to test out all of them to make sure there's not any big issues.
May 4, 2013 at 7:39 PM
I tested the recent changes with all 8 ACs, not seeing any glaring issues. Won't know if there's any numbers off until more people test.
May 5, 2013 at 9:07 PM
Cool -- I'm going to post the latest on the live site, probably sometime today, along with a couple other minor bug fixes and item data tweaks.