BSData / adeptus-titanicus

Adeptus Titanicus
http://battlescribedata.appspot.com/#/repo/adeptus-titanicus
15 stars 19 forks source link

Update Household.cat #199

Closed Prioress closed 2 years ago

Prioress commented 2 years ago

Sorry, I am new to this. I have been trying to fix a bug that prevents users from selecting lance banners in Household Rosters. I probably broke other things too, but I got that to work. Mind checking over my work?

zopha commented 2 years ago

I @Deviltree was going to work on this, I'm not sure if he's made any progress but I'll try to have give it a look over. Hopefully it's all good and get it merged, will be good to give the knights some love.

Prioress commented 2 years ago

Hey! I hope I didn’t break anything. I mostly just corrected the errors in the Household file and linked the banners so they would be selectable. Hopefully nothing broke.

On Tuesday, January 4, 2022, Niall Kearney @.***> wrote:

I @Deviltree https://github.com/Deviltree was going to work on this, I'm not sure if he's made any progress but I'll try to have give it a look over. Hopefully it's all good and get it merged, will be good to give the knights some love.

— Reply to this email directly, view it on GitHub https://github.com/BSData/adeptus-titanicus/pull/199#issuecomment-1005006574, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXBP5AXNH3X7I36FEQAWSPLUUMSIJANCNFSM5KYD5LUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

zopha commented 2 years ago

Getting an error when I try to build a roster :(

Does it work for you?

image

Might have found the issue but I'm not sure why,

image

The XML line is present at the begining of the file where it should be but also appears in a rule description. Looking at it again and it looks like the first 24 lines of code have been duplicated, the categoryEntries start again at line 26.

notepad++_92DCRAN2S0

You can fix it by using a text editor to remove lines 2 to 23 inclusive and save it as a new commit. The lines highlighted here -

image

sorry, lots of large images.

What did you use to edit the file?

Prioress commented 2 years ago

I used the Battlescribe Data Editor ^^;. I’ll get to work on it as soon as I can!

On Saturday, January 15, 2022, Niall Kearney @.***> wrote:

Getting an error when I try to build a roster :(

Does it work for you?

[image: image] https://user-images.githubusercontent.com/1666415/149615642-7b342dfa-4e85-4159-b02b-c03ebe0b981f.png

Might have found the issue but I'm not sure why,

[image: image] https://user-images.githubusercontent.com/1666415/149615861-908a2c08-22a5-4898-b12c-10cf466b02d1.png

The XML line is present at the begining of the file where it should be but also appears in a rule description. Looking at it again and it looks like the first 24 lines of code have been duplicated, the categoryEntries start again at line 26.

[image: notepad++_92DCRAN2S0] https://user-images.githubusercontent.com/1666415/149616017-120e7e09-89cb-4429-a2cc-8c5b9b6b977c.png

You can fix it by using a text editor to remove lines 2 to 23 inclusive and save it as a new commit. The lines highlighted here -

[image: image] https://user-images.githubusercontent.com/1666415/149616181-dd8a90df-ef26-47ae-b3b9-444fb58da652.png

sorry, lots of large images.

What did you use to edit the file?

— Reply to this email directly, view it on GitHub https://github.com/BSData/adeptus-titanicus/pull/199#issuecomment-1013647286, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXBP5AREAGUKPCCYWD72IT3UWEZZRANCNFSM5KYD5LUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

zopha commented 2 years ago

It must have corrupted it in someway, can you open the file on your system? I downloaded your version from github and BS crashes when it tries to load it.

Prioress commented 2 years ago

Yeah I can, I basically always test with Battlescribe open to check if it works. I probably messed something up though.

On Sunday, January 16, 2022, Niall Kearney @.***> wrote:

It must have corrupted it in someway, can you open the file on your system? I downloaded your version from github and BS crashes when it tries to load it.

— Reply to this email directly, view it on GitHub https://github.com/BSData/adeptus-titanicus/pull/199#issuecomment-1013883011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXBP5AXVAF3JHXXEDSEY2Z3UWLGSNANCNFSM5KYD5LUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

jorisvangestel commented 2 years ago

As @zopha mentioned, once the duplicated code in lines 1 through 23 is removed the file loads. While your change does work when this is done, I think you have misunderstood how a Household roster is created (see the comments on #195). This change would indeed break a lot of things because what you have basically done is duplicated all Lance banner entries so they can be directly added to the Household, instead of a Lance Force selection. There are several restrictions on banners when placed in a Lance which can not be enforced if these banners are not part of a Lance Force.

While I understand the confusion about how to create a Lance and add banners to it, imho this is not the correct way of addressing this issue.

Prioress commented 2 years ago

No stress if you want to discard the change, I’ve learned a LOT about coding recently and I can easily see that this was not the best solution. Plus I now have a lot more experience with git. My next pull will be a lot more substantial. Probably. If this works in C I have not checked it since I started learnint >_<

On Thursday, February 3, 2022, MoDLegion @.***> wrote:

As @zopha https://github.com/zopha mentioned, once the duplicated code in lines 1 through 23 is removed the file loads. While your change does work when this is done, I think you have misunderstood how a Household roster is created (see the comments on #195 https://github.com/BSData/adeptus-titanicus/issues/195). This change would indeed break a lot of things because what you have basically done is duplicated all Lance banner entries so they can be directly added to the Household, instead of a Lance Force selection. There are several restrictions on banners when placed in a Lance which can not be enforced if these banners are not part of a Lance Force.

While I understand the confusion about how to create a Lance and add banners to it, imho this is not the correct way of addressing this issue.

— Reply to this email directly, view it on GitHub https://github.com/BSData/adeptus-titanicus/pull/199#issuecomment-1028683833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXBP5AVXGOK6OXWBGS2KIWTUZIVVVANCNFSM5KYD5LUA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

jorisvangestel commented 2 years ago

Closing this now as per the comments here. The readme has been updated as part of #218 with instructions on how to building a household list works, which will hopefully about similar confusion in the future.