search

CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
9.73k stars 4.05k forks source link

Revert "Bow reload time fix (#73305)" #73482

Closed I-am-Erk closed 2 weeks ago

I-am-Erk commented 2 weeks ago

This reverts commit da07b2aa8c97b9a2fd2737afcfdab9f6f549de49.

Summary

None

Purpose of change

73305 is a port of a bright nights pr which in turn was a port of #59977. There's no proper attribution in 73305 for the changes made by BN contributors, which is in violation of our license, but also I'm not sure what's going on here: as far as I can tell the changes made by the BN contributors were to suit the differences in their code base and aren't appropriate for a direct port to DDA. From a scan, it looks like the PR does new things, which then just confuses me because it's framed as a port of a BN change. Regardless, the attribution issue is an urgent one needing reversion and repair.

Describe the solution

Revert, try again if necessary.

Describe alternatives you've considered

None, attribution issues are a big deal.

Testing

n/a

I-am-Erk commented 2 weeks ago

@osuphobia pinging you for your awareness, in case the PR mention doesn't do it.

osuphobia commented 2 weeks ago

Sorry, I was not meant to violate the license. I'm still not familiar enough with github and open source software. The first half of the BN PR was a port of https://github.com/CleverRaven/Cataclysm-DDA/pull/59977, and the second half of it was to deal with https://github.com/CleverRaven/Cataclysm-DDA/commit/d94eea7b5acdb915e6c9908ff72e74456def0e61 which is still an issue in dda. Because the first half is not needed, and the functions in BN and in dda are quite different, I didn't directly copy the lines in BN, but revised our functions in the light of it.

Can you please tell me how to make a proper attribution in this situation?

And as the inconsistency pointed out in https://github.com/CleverRaven/Cataclysm-DDA/issues/73354 needs to be solved, I will not try to make a PR again until I found a perfect solution. Then, can you please also reopen https://github.com/CleverRaven/Cataclysm-DDA/issues/50571 https://github.com/CleverRaven/Cataclysm-DDA/issues/54997?

I-am-Erk commented 2 weeks ago

There are a bunch of ways to do it. Probably, you could just commit the stuff you've based on BN work under a commit named something like "adapted from solution by (username)". That's how I'd do it. You could also credit them in a comment or something but commit history is our preferred way of storing credit. Then make sure to ask mergers not to squash.

No worries about the error, it happens. This isn't a hard feelings revert, just a "this must be done right away" revert.