rpherbig / dr-scripts

A series of Lich 5 (https://github.com/elanthia-online/lich-5) scripts for use with DragonRealms (http://www.play.net/dr/). Donations are welcome (http://www.paypal.me/rcuhljr)!
GNU General Public License v2.0
52 stars 177 forks source link

4.6.49 lich and updated dr-scripts are not working together #3175

Closed Sarvatt closed 5 years ago

Sarvatt commented 5 years ago

Just got a report that someone not using the lich fork was running into no scripts working and switching to the fork fixed it on ruby 2.5.1, there may be a logic error in https://github.com/rpherbig/dr-scripts/commit/78e3d03a3b09e79b3abd8ac801ca6eb267e5ab4e#diff-9b7b8b61093358062f1ab4ac37b348de , $MODERN_LICH should be true on 4.6.49 also where trust was removed for newer ruby also? stock lich doesn't work out of the box unless someone is using the fork right now.

rcuhljr commented 5 years ago

Is it time to kill support for non fork with dependency?

Sarvatt commented 5 years ago

i submitted that as https://github.com/rpherbig/dr-scripts/pull/2929 but got pushback :)

rcuhljr commented 5 years ago

But if we don't go that way, I think LICH_VERSION =~ /^(\d+)\.(\d+)\./ should have an f in it for fork. something like LICH_VERSION =~ /^(\d+)\.(\d+)\.\d+f./

rcuhljr commented 5 years ago

I haven't given this a lot of thought, so I could easily be convinced to do something else (I don't have a strong preference right now).

That's some vicious push back :P

Sarvatt commented 5 years ago

not caring enough to convince 💃

Sarvatt commented 5 years ago

it could have been implemented better, i am the ruby noob who was just trying to fix https://github.com/rpherbig/dr-scripts/issues/2000 getting in over my head.

jandersson commented 5 years ago

But if we don't go that way, I think LICH_VERSION =~ /^(\d+)\.(\d+)\./ should have an f in it for fork. something like LICH_VERSION =~ /^(\d+)\.(\d+)\.\d+f./

I initially had the check as LICH_VERSION =~ /^(\d+)\.(\d+).*(f)/ then removed it because I did not realize there was a problem.

rpherbig commented 5 years ago

I'm swamped, can someone ELI5 this for me?

jandersson commented 5 years ago

This should be fixed by #3180 and #3179 . The issue was not checking non-fork lich version numbers correctly. The assumption was that everyone was on the fork, which was not a good assumption

Sarvatt commented 5 years ago

thanks Jonas!