Skip to content
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

Jump to bottom

[Patch] removing most of the >> as / related to Money values #1149

Closed
DorpsGek opened this issue Aug 22, 2007 · 6 comments
Closed

[Patch] removing most of the >> as / related to Money values #1149

DorpsGek opened this issue Aug 22, 2007 · 6 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

Noldo opened the ticket and wrote:

As a side product of trying to make Money into class I ran into ">> 4" and similar ones applied to values of type Money. I feel that these should be changed to use / because it's more natural way for a human to read and also having a bit shift operator on a class that is an abstraction of money later doesn't seem to fit.

Attachments

Reported version: trunk
Operating system: All

This issue was imported from FlySpray: https://bugs.openttd.org/task/1149
@DorpsGek
Copy link
Member Author

Noldo wrote:

Updated a bit after comments from peter1138.
Also fixed few things caused by overeagerness to remove bitshifts.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149#comment1937

@DorpsGek
Copy link
Member Author

Noldo wrote:

Lynx is not my friend.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149#comment1938

@DorpsGek
Copy link
Member Author

Noldo wrote:

Now with correct number of spaces.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149#comment1957

@DorpsGek
Copy link
Member Author

Noldo wrote:

Unified vehicle gui required a reaction.

Attachments

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149#comment2012

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 4, 2007

TrueBrain wrote:

Although the patch in fact looks nice, I don't see the usage of converting all the >> to /. It only tempt people to use non-power-of-two values, which will cause big slowdowns. For that reason, this patch is rejected for now. Who knows it will ever be applied, but currently there is no real gain, only a step back.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149#comment2594

@DorpsGek
Copy link
Member Author

DorpsGek commented Nov 4, 2007

TrueBrain closed the ticket.

Reason for closing: Won't implement

Patch rejected. Currently no real gain.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1149

@DorpsGek DorpsGek closed this as completed Nov 4, 2007
@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@DorpsGek