This is a list of all comments for SWGEMU-375. Review Summary: commiting General Comment by Cebot on 24 August 2012, 16:35 http://www.swgemu.com/fisheye/cru/SWGEMU-375#c807 The basic issue with city advancement is that the "isCityRankCapped" was returning "true" too often because it wasn't comparing the cities by rank, just the total count. This one is "the sooner the better" because the longer we wait to fix it, the more cities get stuck, and it won't untangle itself very well. General Comment by Cebot on 25 August 2012, 00:59 http://www.swgemu.com/fisheye/cru/SWGEMU-375#c808 After much discussion about this tonight, I'm also thinking the check for totalCities > maxCities (the number of outposts, really) should go away. Just because the planet somehow got over the limit doesn't mean no one should be able to progress.I updated the patch to reflect this change. Reply by TheAnswer on 26 August 2012, 18:16 > who did you discuss this with? Reply by Cebot on 26 August 2012, 20:42 > Just other community members - nothing official. But it > doesn't make sense to stop an entire planet's worth of city > progression just because there are too many cities total. General Comment by Xaroth Brook on 08 October 2012, 15:03 http://www.swgemu.com/fisheye/cru/SWGEMU-375#c850 Right, re-did this with the information I could grab from the scrapbook due to not being able to upload files, the patch, pasted last As per the scrapbook: The number of player cities that can be placed is capped. The 3 main worlds of Corellia, Naboo, and Tatooine are able to hold 20 and the moons and adventure worlds of Talus, Rori, Dantooine, and Lok can hold up to 50 cities. It is important to note that there is also a cap on the number of cities that can achieve a certain rank. While the core worlds of Corellia, Naboo, and Tatooine can have a total of 20 cities, only 15 can be rank 3 or above and only 10 can be rank 4 or above. The moons/adventure planets of Talus, Rori, Dantooine, and Lok can have 30 cities above rank 3 and 20 above rank 4. To accomplish this: - Scrap the check for max cities, at all point we're checking for the rank at hand, which is equal or lower than the planet limit anyhow. - Check for cities of equal or greater rank than being checked, as per the scrapbook. - Double-check at the end. The patch: (note, NEEDS TESTING, I don't have a full dev env running atm, so cannot do full testing.......) Downloadable: http://rawr.xaroth.nl/xar/CityManagerImplementation.cpp.patch Index: MMOCoreORB/src/server/zone/managers/city/CityManagerImplementation.cpp =================================================================== --- MMOCoreORB/src/server/zone/managers/city/CityManagerImplementation.cpp (revision 5901) +++ MMOCoreORB/src/server/zone/managers/city/CityManagerImplementation.cpp (working copy) @@ -186,9 +186,9 @@ bool CityManagerImplementation::isCityRankCapped(const String& planetName, byte rank) { Vector* citiesAllowed = &citiesAllowedPerRank.get(planetName); - byte maxCities = citiesAllowed->get(0); byte maxAtRank = citiesAllowed->get(rank - 1); - byte totalCities = 0; + // http://www.swgemu.com/archive/scrapbookv51/data/20070127193144/index.html for information + byte totalAtRank = 0; Locker _lock(_this.get()); @@ -200,12 +200,14 @@ if (cityZone == NULL || cityZone->getZoneName() != planetName) continue; - if (++totalCities > maxCities || totalCities > maxAtRank) { - return true; + if (city->getCityRank() >= rank) { + if (++totalAtRank >= maxAtRank) { + return true; + } } } - - return false; + + return (totalAtRank >= maxAtRank); } bool CityManagerImplementation::validateCityInRange(CreatureObject* creature, Zone* zone, float x, float y) { Reply by Cebot on 08 October 2012, 22:10 > My original patch has been tested, and while yours is > slightly more efficient (breaks out of the loop as soon as > totalAtRank > maxAtRank), it's also flawed because it > compares city >= rank, not city == rank. The list of rank > caps is per rank, and cities of a higher rank are irrelevant. Reply by Xaroth Brook on 09 October 2012, 00:07 > I think you missed out my reasoning, as per the scrapbook: > > ---start quote--- > The number of player cities that can be placed is capped. > The 3 main worlds of Corellia, Naboo, and Tatooine are able > to hold 20 and the moons and adventure worlds of Talus, > Rori, Dantooine, and Lok can hold up to 50 cities. It is > important to note that there is also a cap on the number of > cities that can achieve a certain rank. While the core > worlds of Corellia, Naboo, and Tatooine can have a total of > 20 cities, only 15 can be rank 3 or above and only 10 can > be rank 4 or above. The moons/adventure planets of Talus, > Rori, Dantooine, and Lok can have 30 cities above rank 3 > and 20 above rank 4 > ---end quote--- > > it clearly states, only 15 can be _rank 3 or above_, and > only 10 _can be rank 4 or above_. > Your test only tests at the current rank, which is flawed, > as when there are 10 rank 4(or 5) cities, none are allowed > to progress from rank 3 to 4, while a rank 4 is able to > progress to 5 > > With your method we would be able to have a total of 100+ > cities on a planet, as 1) there is no limit being > calculated, and 2) you could have 10 rank 4 cities AND 10 > rank 5 cities. > > Sources: > http://www.swgemu.com/archive/scrapbookv51/data/20070127193144/index.html > http://www.swgemu.com/archive/scrapbookv51/data/20070127193158/index.html > http://swg.wikia.com/wiki/Player_city#Player_city_rank Reply by Xaroth Brook on 09 October 2012, 05:59 > Actually, my statement about your version is flawed, i > should stop typing when I just wake up... > > Take this case: > > planet x has a cap of 50/50/30/20/20 > if there are 20 towns at rank 5, none are able to progres > from 3 to 4, in your test, this fails, as there are 0 > rank 4 towns, and that code would allow progression. > If there are 20 towns at rank 5 still, and 10 at rank 3, > none should be allowed to upgrade from 2 to 3, again, > your code would not catch this. > > I hope this clears it up even more. Reply by Cebot on 09 October 2012, 06:57 > Damn. I hate it when I'm wrong. :) /deepbow > > I was mis-interpreting that as the number per rank, > obviously. General Comment by elpete on 10 October 2012, 08:12 http://www.swgemu.com/fisheye/cru/SWGEMU-375#c857 I 'think' that this is also called when placing a city deed. So this looks like it fixes the expansion issue but the check to see if the planet is allowed anymore cities is bypassed.... which might be a totally different issue. Just brainstorming here but maybe two functions are needed? 1) To see if you can place a city to use when placing the city hall deed 2) To see if an existing city can expand. Reply by Xaroth Brook on 10 October 2012, 11:26 > well, if you check for rank 1, you do the same check as the > max-city check did, as _every_ town will be either rank 1 or > higher... Reply by elpete on 10 October 2012, 12:57 > OK. I just realized you have a link to a patch in one of > your comments. I'm guessing that's the one your'e talking > about. Reply by Xaroth Brook on 10 October 2012, 14:34 > You are very much correct General Comment by TheAnswer on 12 October 2012, 08:52 http://www.swgemu.com/fisheye/cru/SWGEMU-375#c866 anchored xaroths patch --- ID: SWGEMU-375 http://www.swgemu.com/fisheye/cru/SWGEMU-375 Title: City Advancement Statement of Objectives: Allow cities to advance properly State: Closed Summary: commiting Author: Cebot Moderator: TheAnswer Reviewers: (8 active, 0 completed*) Itac Loshult cRush dannuic oru Kyle Xaroth Brook elpete