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
Changed things to do with Shroud to WRange. Updated Utility. #4694
Conversation
@@ -24,6 +24,7 @@ public struct WPos | |||
public WPos(WRange x, WRange y, WRange z) { X = x.Range; Y = y.Range; Z = z.Range; } | |||
|
|||
public static readonly WPos Zero = new WPos(0, 0, 0); | |||
public static WPos FromCPos(CPos cell) { return new WPos(cell.X, cell.Y, 0); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the definition of our coordinate system. There is a scaling factor and offset between cell and world units, so you can't just convert between the two like this. Use CPos.CenterPosition
instead.
I swear I fixed that first one earlier today, maybe I just forgot to push. |
Your strange reveal circles are because your final filtering check is done in cell units. The |
{ | ||
foreach (var q in FindVisibleTiles(world, center, range)) | ||
explored[q.X, q.Y] = true; | ||
|
||
var box = new Rectangle(center.X - range, center.Y - range, 2*range + 1, 2*range + 1); | ||
var box = new Rectangle(center.X - (range.Range / 1024), center.Y - (range.Range / 1024), 2 * (range.Range / 1024) + 1, 2 * (range.Range / 1024) + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should define a variable for range.Range / 1024
(you may also want to round up using (range.Range + 1023) / 1024
) to avoid unnecessary duplication.
Good job generalizing this ugly low-level code :) Tentative 👍 once you
|
As we discussed in IRC last week, you will still need to remove the unrelated changes that --upgrade-mod introduced. You will also need to rebase on to the latest bleed. This is a fundamental change, so I would like to see a :+2: from another reviewer before we merge this. |
Alright, I think this is finally done... |
👎 crashes the RA shellmap:
|
Fixed. |
👍 can't find any regressions. Checked both shroud reveal and gap generator. |
Changed things to do with Shroud to WRange. Updated Utility. Closes #4679
Yay! Finally! Anyway, I will not be able to complete the d2k updates, I think I might as well just scrap the PR, as I am no longer interested in re-balancing the mod, as no one plays it. |
That would be a shame. The impression I have is that one of the main reasons people don't play D2K is that our mod plays nothing like the original, but is instead a "ra with sand" style spam-fest. I was hoping that your balance changes would address that by resetting the balance and therefore gameplay closer to the original game. |
Others seem to want to take on that task http://www.sleipnirstuff.com/forum/viewtopic.php?f=83&t=16465 |
Closes #4679